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

Create a Chakra Select component #8630

Closed
Tracked by #8632
pettinarip opened this issue Nov 17, 2022 · 23 comments · Fixed by #9856
Closed
Tracked by #8632

Create a Chakra Select component #8630

pettinarip opened this issue Nov 17, 2022 · 23 comments · Fixed by #9856
Assignees
Labels
archived: chakra-migration feature ✨ This is enhancing something existing or creating something new

Comments

@pettinarip
Copy link
Member

pettinarip commented Nov 17, 2022

This is part of our 2nd wave of migration of the Implement UI library epic.

Is your feature request related to a problem? Please describe.

Right now we have a Select component that is styled using styled components and uses react-select.

Describe the solution you'd like

  • Deprecate the usage of the current component
  • Create a new component called Select under src/components that is wrapped and styled using Chakra.
  • Create the theme for the Chakra Select component
  • Replace the old with the Chakra Select component in the entire codebase

Additional context

Be aware of this: there are cases where we are adding more styles to the base styles of the Select.

@pettinarip pettinarip added feature ✨ This is enhancing something existing or creating something new archived: chakra-migration labels Nov 17, 2022
@pettinarip pettinarip self-assigned this Nov 17, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2023

This issue is stale because it has been open 45 days with no activity.

@github-actions github-actions bot added the Status: Stale This issue is stale because it has been open 30 days with no activity. label Jan 2, 2023
@pettinarip pettinarip removed their assignment Jan 24, 2023
@pettinarip pettinarip removed the Status: Stale This issue is stale because it has been open 30 days with no activity. label Jan 24, 2023
@github-actions
Copy link
Contributor

This issue is stale because it has been open 45 days with no activity.

@github-actions github-actions bot added the Status: Stale This issue is stale because it has been open 30 days with no activity. label Mar 13, 2023
@pettinarip pettinarip self-assigned this Apr 4, 2023
@TylerAPfledderer
Copy link
Contributor

TylerAPfledderer commented Apr 4, 2023

@pettinarip you requested I continue the conversation here about the use of ZagJS to create a custom select for properly handling the styling from the new DS.

I would argue for the use of Zag over react-select. If I'm not mistaken, the bundle size is smaller unless react-select is tree-shakeable

DISCLAIMER: I've not looked far enough into this yet to conclude otherwise.

@pettinarip
Copy link
Member Author

@TylerAPfledderer @nloureiro

Currently I see that we have 2 type of selects/dropdowns:

1. StyledSelect
This is a react-select wrapper. We are using this component in just 4 places:

  • EthExchanges
  • WalletTable
  • Layer2Onboard
  • StakingLaunchpadWidget

In neither of them, we are using complex logic (autocomplete, multiselection, etc), just the basic native select features.

image

2. ButtonDropdown
This is a custom component, built with a button and a ul with an animation. This one is mainly being used in the templates to display related links to the page.

image


My biggest concern is related to the amount and size of the libs that we are adding. I was trying to think if there was a way we could simplify these two given that the features we require for them are practically none.

In my mind, I have these two ideas:

  • Avoid using a lib, just use the native select. We are going to simplify everything but at the cost of losing customization (on the options list). I don't think this is even an option, right @nloureiro?
  • Using a lighter lib. I heard good opinions about using https://www.downshift-js.com/ which is just the logic to build a custom select/dropdown component. With it, we could build the two components previously mentioned and also have a robust logic to scale in the future (in case we need autocomplete and other features).

Comparing sizes:

  • zagjs requires 2 imports:
    • @zag-js/react 9.4kB
    • @zag-js/select 28.3kB
  • react-select 28.6kB
  • downshift 13.8kB

@pettinarip pettinarip removed Status: Stale This issue is stale because it has been open 30 days with no activity. 30web3 labels Apr 5, 2023
@TylerAPfledderer
Copy link
Contributor

TylerAPfledderer commented Apr 6, 2023

@pettinarip I think downshift can be considered here, keeping aria patterns in mind. Threw together a CSB comparing Zag and Downshift select implementation. Basically took the Zag structure I already made and replace the machine logic with the logic from downshift's useSelect hook. (Some [tiny] styling adjustments might still be needed)

To make it easier to view from dev tools, here is the Zag example content elements that are in a portal. (What it looks like on my end in Firefox)
image

Wouldn't the different lib sizes in development be smaller than what you listed since they all are tree-shakeable?

@nloureiro
Copy link
Contributor

Jumping into the mix now.

Help me understand why we can´t use the chakra default.

@pettinarip, you divide them into two types of select
1- to filter content
2 - to navigate, this case is only left subpages navigation.

My take:
1- let's use the default for this, and I can adapt the design to the chakra one. I might need a bit more time to test this, but the first assessment indicates that it is doable. 2- for links, we should never use a and a list of

  • with links that are better semantic and better for SEO.

    Am I missing anything here?

  • @nloureiro
    Copy link
    Contributor

    Jumping into the mix now.

    Help me understand why we can´t use the chakra default.

    @pettinarip, you divide them into two types of select 1- to filter content 2 - to navigate, this case is only left subpages navigation.

    My take: 1- let's use the default for this, and I can adapt the design to the chakra one. I might need a bit more time to test this, but the first assessment indicates that it is doable. 2- for links, we should never use a and a list of

    • with links that are better semantic and better for SEO.
      Am I missing anything here?

    [replying to myself]

    Using the chakra default, we lose the ability to "type to filter", correct?

    @TylerAPfledderer
    Copy link
    Contributor

    TylerAPfledderer commented Apr 6, 2023

    @nloureiro I'm thinking of the styling first. And the native select (or using Chakra's select component) does not give you the flexibility to style the dropdown container around the options and you wouldn't be able to do the styling you have the new DS.

    Side note: OpenUI (a W3 community) has proposed a new element called selectmenu to address the styling limitations. Based on article dates I've found, it's been a proposal for at least a year.

    As to a filter feature, there is not one for this type of dropdown in Chakra. This type is being referred to as a Combobox which we are looking to add in version 3. (I don't remember why it is not being considered for the current Chakra other than possible performance and JS payload concerns)

    @nloureiro
    Copy link
    Contributor

    @nloureiro I'm thinking of the styling first. And the native select (or using Chakra's select component) does not give you the flexibility to style the dropdown container around the options and you wouldn't be able to do the styling you have the new DS.

    Side note: OpenUI (a W3 community) has proposed a new element called selectmenu to address the styling limitations. Based on article dates I've found, it's been a proposal for at least a year.

    As to a filter feature, there is not one for this type of dropdown in Chakra. This type is being referred to as a Combobox which we are looking to add in version 3. (I don't remember why it is not being considered for the current Chakra other than possible performance and JS payload concerns)

    When we speak of the chakra one, is this one?
    https://chakra-ui.com/docs/components/select/usage

    right?

    @TylerAPfledderer
    Copy link
    Contributor

    @nloureiro Correct!

    @pettinarip
    Copy link
    Member Author

    Awesome @TylerAPfledderer! looks great in the codesandbox.

    To make it easier to view from dev tools, here is the Zag example content elements that are in a portal. (What it looks like on my end in Firefox)

    Couldn't follow you on this. Why did you share that image? to see the aria attrs generated by Zag?

    Wouldn't the different lib sizes in development be smaller than what you listed since they all are tree-shakeable?

    Hmm...not sure. Perhaps. However, I don't think there will be that much to "shake" from these libs since they are pretty small and very focused on just one thing.

    @pettinarip
    Copy link
    Member Author

    @nloureiro

    2- for links, we should never use a and a list of

    • with links that are better semantic and better for SEO.

    Do you mean that we will not use ButtonDropdown to handle that navigation anymore?

    @TylerAPfledderer
    Copy link
    Contributor

    TylerAPfledderer commented Apr 7, 2023

    @pettinarip

    Couldn't follow you on this. Why did you share that image? to see the aria attrs generated by Zag?

    Yes, to identify the attributes applied from Zag versus the attributes applied from downshift in the similar structure.

    Since the Zag version of the options list sits in a portal, I figure I would provide the screenshot to easily show it. 😁

    @nloureiro
    Copy link
    Contributor

    @nloureiro

    2- for links, we should never use a and a list of

    • with links that are better semantic and better for SEO.

    Do you mean that we will not use ButtonDropdown to handle that navigation anymore?

    Not sure what is ButtonDropdown
    is this the ButtonDropdown?
    Screen Shot 2023-04-09 10 38 35 PM

    @pettinarip
    Copy link
    Member Author

    Ok, an update on this front.

    Talking with @nloureiro we were converging on the idea of using the Chakra UI components as much as we can. Having said that, in order to reduce the number of libs we currently have, is to do the following:

    • Selects: use the Chakra Select component
      The downside is that we are going to lose the ability to customize and filter the options.

    • Dropdowns (our ButtonDropdown): use the Chakra Menu component

    With this approach, we will be able to remove the react-select lib for now.

    @TylerAPfledderer what do you think about this? I'm sorry for not catching this before you did all the work on the codesandboxes 🙏🏼

    @TylerAPfledderer
    Copy link
    Contributor

    TylerAPfledderer commented Apr 12, 2023

    @pettinarip For the Select, what will be lost is the design from the DS when the options are visible. (Is that what you meant by customize?)

    @pettinarip
    Copy link
    Member Author

    @pettinarip For the Select, what will also be lost is the design from the DS when the options are visible. (Is that what you meant by customize?)

    Correct

    @TylerAPfledderer
    Copy link
    Contributor

    @pettinarip That's good for me!

    If you haven't noticed yet, I do have a PR up in draft for the form components. Would you want a custom Select component to house logic and handle the listing of options and their data? Or just create the theming and a story file?

    @pettinarip
    Copy link
    Member Author

    pettinarip commented Apr 12, 2023

    @TylerAPfledderer Cool, thanks!

    If you haven't noticed yet, I do have a PR up in draft for the form components.

    Nice! I've seen the PR created but haven't reviewed it yet.

    Would you want a custom Select component to house logic and handle the listing of options and their data? Or just create the theming and a story file?

    Not sure if you've detected a specific case where we have special logic around the selects we have. I haven't. Since we don't need anything special, lets just create the theme for the Chakra Select.

    @TylerAPfledderer
    Copy link
    Contributor

    @pettinarip I did not replace the instances of react-select in the PR for the form component themes. There is logic under the hood of the react-select component to keep the flow compact, including the use of a custom onChange that returns data of the selected option that was sent to it, instead of the DOM event object.

    After viewing this logic again, it seems that a custom component would be helpful in place of react-select, else there could be a lot of bloat and repetitive flow.

    Instances:

    Were you looking to simplify the current logic?

    @pettinarip
    Copy link
    Member Author

    Ok, so you have implemented the new styles on the Chakra Select but not replaced it on the codebase. That fine 👍🏼

    In terms of abstracting that logic I'm ok as well. Do you have in mind creating a new folder under /components/Select?

    Were you looking to simplify the current logic?

    Wasn't thinking about doing that tbh but if that helps us to keep things easier to use, then lets do it.

    @TylerAPfledderer
    Copy link
    Contributor

    TylerAPfledderer commented Apr 17, 2023

    In terms of abstracting that logic I'm ok as well. Do you have in mind creating a new folder under /components/Select?

    @pettinarip yes. I could try and leave the logic passed to the component in each page as is and then in this new Select component mimic the props that are currently being used from react-select.

    @pettinarip
    Copy link
    Member Author

    @TylerAPfledderer perfect. Yea, that sounds great.

    @pettinarip pettinarip changed the title Create a Chakra Select component using react-select Create a Chakra Select component Jun 19, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    archived: chakra-migration feature ✨ This is enhancing something existing or creating something new
    Projects
    None yet
    Development

    Successfully merging a pull request may close this issue.

    4 participants