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

Upgrade react select #1482

Merged
merged 7 commits into from
Mar 15, 2022
Merged

Upgrade react select #1482

merged 7 commits into from
Mar 15, 2022

Conversation

joverlee521
Copy link
Contributor

@joverlee521 joverlee521 commented Mar 9, 2022

Upgrades our react-select package to version 5.2.2 and replaces the react-virtualized-select package with our own virtualized menu list built with react-virtualized components.

Note that the upgrade of react-select did include style changes. I did not dig into the style changes, but please let me know if we want the styles to match the old version.

Resolves #1467 and ##1453.

@nextstrain-bot nextstrain-bot temporarily deployed to auspice-upgrade-react-s-amd7fu March 9, 2022 20:02 Inactive
@joverlee521
Copy link
Contributor Author

joverlee521 commented Mar 9, 2022

A good test of the dynamic options is searching for "submitting lab" in the seasonal flu build of the test app.

I'm currently looking into the bundle size issue. Fixed in latest push.

We have been using v1 which is not longer maintained. We want to use
new features available such as option groups.
The package `react-virtualized-select` is specifically built for
v1 of `react-select` and is no longer maintained. Since we have upgraded
`react-select`, this package will no longer work.

Keep `react-virtualized` to build our own virtualized menu component
to use with `react-select`.
With the complete rewrite of `react-select` from v1 to v2, there are a
couple changes to the props of the `Select` component that affect us:

1. `clearable`, `multi`, `searchable` props have been renamed to
`isClearable`, `isMulti`, `isSearchable`
2. `valueKey` has been removed so we must provide `getOptionValue` for
potential Symbol values
3. `value` no longer accepts a simple string. It now always expects
an array of options or a single options object.

The following commit will address the `Select` component from
`react-virtualized-select` used for the data filter.
Replaces the `Select` component including an `async` flag from
`react-virtualized-select` with the `AsyncSelect` component from the
updated `react-select` package.

Keeps the equivalent props in the updated component, but does not
include the virtualized MenuList component. This will be addressed in
the next commit.
@joverlee521 joverlee521 force-pushed the upgrade-react-select branch from 0a50744 to 07811b5 Compare March 9, 2022 21:46
@joverlee521 joverlee521 temporarily deployed to auspice-upgrade-react-s-amd7fu March 9, 2022 21:46 Inactive
@joverlee521
Copy link
Contributor Author

The test PR in the nextstrain.org repo is failing because I did not realize that the react-select and react-virtualized-select packages are used in nextstrain.org as well. I'll work on updating the package in the nextstrain.org repo.

@joverlee521
Copy link
Contributor Author

The latest changes can now be tested with the SARS-CoV-2 builds that include the originating/submitting labs at https://nextstrain-s-upgrade-re-8ecxpi.herokuapp.com/staging/ncov/gisaid/trial/add_authors/global

@jameshadfield
Copy link
Member

Thanks @joverlee521! Haven't dived into the code yet but here's feedback comparing nextstrain-s-upgrade-re-8ecxpi.herokuapp.com against nextstrain.org

  • Suggestion text (e.g. "Type filter query here...") is now rgb(128, 128, 128); but should be #aaa (which is rgb(170, 170, 170);)
  • Cell size looks great!
  • Aesthetically I prefer the filled in down icon compared to the new vertical line & arrow outline, but if this is hard to change in react select I wouldn't be too concerned.
  • Performance seems good (for the filter dropdown, which is the biggest we have)

@emmahodcroft
Copy link
Member

I tried this out this morning and it looks good to me! Even for the very long lists, as James said, the performance seemed good and not too laggy. I think the names wrap well and look really nice and tidy!
I had some fun exploring some of the interesting lab names we have in there... I guess with free-text fields you're always going to have some entries that are... creative! 🙃

Add a VirtualizedMenuList component to replace the MenuList component of
the react-select package for the data filter dropdown.

This was added to replace the react-virtualized-select package that is
no longer maintained an incompatible with the latest version of
react-select.
Uses `CellMeasurer` to measure individual row heights for each option
to accomodate long string options.
@joverlee521 joverlee521 force-pushed the upgrade-react-select branch from 07811b5 to dca4585 Compare March 12, 2022 01:02
@joverlee521 joverlee521 temporarily deployed to auspice-upgrade-react-s-amd7fu March 12, 2022 01:02 Inactive
Replaces all uses of the `Select` component from `react-select` with
our own `CustomSelect` component to centralize any customizations that
we would like to be shared across all dropdowns.

Does not change any functionality of the `Select` component and keeps
all passed props, which allows individual customizations to override
our default customizations.

Since the data filter is the only component that uses `AsyncSelect`,
I decided to only import the custom styles and not create another
custom component.
@joverlee521 joverlee521 force-pushed the upgrade-react-select branch from dca4585 to 561c1fd Compare March 12, 2022 01:10
@joverlee521 joverlee521 temporarily deployed to auspice-upgrade-react-s-amd7fu March 12, 2022 01:10 Inactive
@joverlee521
Copy link
Contributor Author

@jameshadfield The latest commit (561c1fd) centralizes the Select component into our own CustomSelect component that makes it easier to make broad style changes across all drop downs.

I used this to make the styles tweaks of changing the placeholder font color and replacing the drop down arrow. I also updated the dropdown menu to not have a top margin so there's not a weird gap between the input box and the drop down.

Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @joverlee521. Thanks for addressing the styling -- I remember trying to combine styled-components and react-select before they had the components prop and it wasn't pretty! This is much better.

@joverlee521 joverlee521 merged commit 15b9ffc into master Mar 15, 2022
@joverlee521 joverlee521 deleted the upgrade-react-select branch March 15, 2022 22:48
jameshadfield added a commit that referenced this pull request Mar 27, 2022
Note that we now have a custom-styled react-select component  via PR #1482.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Upgrade react-select package
4 participants