Renaming Material UI base form controls

[ErikDakoda]

I am going to rename components in vulcan-ui-material/lib/components/forms/base-controls/ because their names conflict with the style sheet names (used for Global theme overrides) of some core MUI components - for example MuiInput.

These components are actually not registered with registerComponent, only exported. This would be a breaking change for anyone who has built custom components based on them using import (our codebase at Etail21 has 6 such instances, for example).

Here are the proposed changes:

• MuiCheckbox => CheckboxBase
• MuiCheckboxGroup => CheckboxGroupBase
• MuiFormControl => FormControlBase
• MuiFormHelper => FormHelper
• MuiInput => InputBase
• MuiPicker => PickerBase
• MuiRadioGroup => RadioGroupBase
• MuiRequiredIndicator => RequiredIndicator
• MuiSelect => SelectBase
• MuiSuggest => SuggestBase
• MuiText => TextBase

Please let me know if you agree with this change or if you would like to suggest an alternative. Thanks!

[ErikDakoda]

Hi @eric-burel @yairtal I would be interested in your feedback.

[eric-burel]

Ping @EloyID
For me no problem at all conceptually, but please just document this in the MIGRATION.md doc if possible as it is a breaking change.

[EloyID]

HI!
For me is extrange because there is a MUI element called ButtonBase https://material-ui.com/api/button-base/ and the base means it almost contains no UI, only the functions.
Maybe sth like ~Control ~MuiInput ~FormInput?
Anyways ~Base is ok, but I think RequiredIndicator and FormHelper should also finish in ~Base, if theres no special difference with the other components

[ErikDakoda]

Hi @EloyID! Thanks for the feedback! Let me explain my logic. The components I am proposing to rename in /forms/base-controls/ to ~Base all have form controls in /forms/controls/ that are based on them. For example Email, Password, and Url are all based on MuiInput (which would be renamed to InputBase).

In my mind EndAdornment, FormControl, FormHelper, RequiredIndicator, StartAdornment are not base controls, but helpers used by all form controls, that's why I propose a different naming convention, and perhaps they should also be in their own directory:

/forms/helpers/:

• MuiFormControl => FormControl
• MuiFormHelper => FormHelper
• MuiRequiredIndicator => RequiredIndicator
• EndAdornment => EndAdornment
• StartAdornment => StartAdornment

Whether to use ~Base for naming - I am not exactly sure, and I take your point that some of those names actually conflict with built-in MUI component names. I don't mind ~Control:

/forms/base-controls/:

• MuiCheckbox => CheckboxControl
• MuiCheckboxGroup => CheckboxGroupControl
• MuiInput => InputControl
• MuiPicker => PickerControl
• MuiRadioGroup => RadioGroupControl
• MuiSelect => SelectControl
• MuiSuggest => SuggestControl
• MuiText => TextControl

...or Form~:

/forms/base-controls/:

• MuiCheckbox => FormCheckbox
• MuiCheckboxGroup => FormCheckboxGroup
• MuiInput => FormInput
• MuiPicker => FormPicker
• MuiRadioGroup => FormRadioGroup
• MuiSelect => FormSelect
• MuiSuggest => FormSuggest
• MuiText => FormText

What is everybody's preference?

[ErikDakoda]

One more thing we may want to do is to register all of these components with registerComponent (instead of using import) so that they can be more easily overridden in each developer's project.

[eric-burel]

@ErikDakoda We are more taking the direction of going backward on this, because that's make them very very difficult to maintain, it creates unseen dependencies based on name etc.

I would except developer with actual custom need to maintain a fork, that's less magic but in the end maybe as efficient.

You might want to check this: VulcanJS/vulcan-next#3
I am trying to figure alternative patterns to get best of both approaches: the ability to replace export basically like we do currently with replaceComponent.

[ErikDakoda]

@eric-burel It is a good idea to test new and more efficient patterns for Vulcan, and once a workable solution is implemented, tested and documented, to roll it out to the community. In the meantime, though, we have existing apps with tens of thousands of lines of code using current patterns that drew us to Vulcan in the first place. I think it's legitimate to make improvements to existing code using those older patterns.

I am only suggesting adding registerComponent for these components at all because I will be touching the code anyway and these are the only components in the entire vulcan:ui-material that don't use that pattern.

I don't think that contradicts possibly moving away from registerComponent in the future.

[eric-burel]

Yeah of course, feel free to apply any modification because in the end you've probably got one of the biggest Vulcan Material UI app currently active, alongside Live for Good. You can reach Eloy directly to discuss that.

I've found back the actual ticket I wanted to share: #2549
I can't find back the migration advices though...

[EloyID]

I prefer Form since we already know they are controls (inputs) but we want to know that we override them for the SmartForm

I'm really not an expert about the idea of import/registerComponent, but an idea would be

• While using Components.SmartForm, we can count on replaceComponent to override the formcomponents
• In the future we can create a pattern with a config function to override the components of the smartform for the whole application

export CustomSmartForm = customizeSmartForm({
components: {Checkbox: MyOwnCheckbox}
})

[ErikDakoda]

Thank you for the feedback, guys, I will move forward with the renaming using the Form~ pattern. I won't add registerComponents for those components because I don't really need it, and it sounds like you guys don't either.

[ErikDakoda]

@eric-burel I have seen "Magical replacement of components" before - but I will re-read it to better understand where you are going with vulcan-npm.

[ErikDakoda]

This was implemented in PRs #2665 and #2666