Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

misc: start migration of Combobox to Tailwind #1942

Merged
merged 1 commit into from
Dec 26, 2024
Merged

Conversation

ansmonjol
Copy link
Collaborator

Context

We're migrating our app to use Tailwind. This PR is a kickoff for the Combobox one.

Description

Many things happens here, from simple TW switch to some UI and logic change, to make sure the UI looks homogeneous

Please note:

  • MultipleComboBoxList also have a change as I set the maxHeight rule in the MUI theme file.
  • I updated react-window lib to get last bugfixes
  • The ComboBoxPopperFactory lose some padding rules as they are now spread more "close" to the elements concerned (ComboboxList and ComboBoxVirtualizedList.tsx)

Next steps

The Combobox TW migration is not done.
I plan to then handle the more "noisy" part by renaming ComboBoxItem into ComboBoxItemWrapper and creating a new ComboBoxItem component that will return a TW wrapper for the exported const Item included in ComboBoxItemWrapper.
This new component is widely used in the app and all imports will be updated

Some rules may need to be placed in the MUI theme file and once done, the same process will be reproduce for the multiple combobox (with some potential other problems in the road)

Let's be honest, this code is crap and should be fixed differently for the long term. We should probably (i) upgrade our MUI package to v6 to (ii) rely more on their new API and internal tools, rather than making everything custom as today.
That will for sure help us removing bunch of code and components we built.

@ansmonjol ansmonjol merged commit efa35ac into main Dec 26, 2024
12 checks passed
@ansmonjol ansmonjol deleted the combobox-tw-part-one branch December 26, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants