noaignite / create-ignite-app

Boilerplate for React with Next.js and MUI
17 stars 2 forks source link

feature/api-and-context #57

Closed maeertin closed 2 years ago

maeertin commented 2 years ago
vincentboiardt commented 2 years ago

Great work!

What's the reasoning behind naming it Cms instead of Settings? It's mainly used for menus, global links etc. which usually comes from the Cms but I feel like "Settings" or something similar is good naming for it?

Regarding the renaming of Checkout to Centra, should we perhaps go the opposite way and name it to something even more generic? If we'd want to create a new context provider for another service we could just import that provider from that package, like @noaignite/react-salesforce. Something like CommerceContext useCommerceHandlers useCommerceState

alexanderflink commented 2 years ago

@vincentboiardt I have discussed these names with Martin and I think the reasonings are:

useCms instead of useSettingsuseSettings doesn't really communicate that these values do come from the cms (or from somewhere outside the app), but rather sounds more like a configuration context that is set up within the app (which could also have been some sort of configuration.json file). I do agree that useCms is not super specific either, but it communicates that this context contains data from elsewhere.

useCentra instead of useCheckout – the states and handlers within this context are very Centra specific. Since we haven't abstracted them into something generic it makes sense for the context to not have a generic name. Also, I have experienced some confusion with new devs as to what useCheckout means – it's easily mistaken for something that only has with the checkout page to do.

vincentboiardt commented 2 years ago

OK! I can agree that we shouldn't use something generic until it's generic.

I see the resoning behind the naming for useCms as well. I saw that Firebase call their concept "Remote Config" https://firebase.google.com/docs/remote-config

Perhaps it makes it easier to differ local config from remote config.

maeertin commented 2 years ago

Hey @vincentboiardt, remote-config feels like a good name! I've now updated with:

const { menus } = useRemoteConfig()

vincentboiardt commented 2 years ago

So haxx 😎