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.

Sign up for GitHub

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

Jump to bottom

feat: build custom react-select component with Chakra [updated] #12065

Merged
merged 21 commits into from
Mar 6, 2024
Merged

feat: build custom react-select component with Chakra [updated] #12065

merged 21 commits into from
Mar 6, 2024

Conversation

TylerAPfledderer
Copy link
Contributor

@TylerAPfledderer TylerAPfledderer commented Jan 30, 2024 •
edited
Loading

Replaces #10542

Part of Implementation of new DS v1 #9548

Description

This PR update the usage of the react-select package.

  • Bumps the react-select dep to latest which includes TypeScript support
  • Creates a custom select component, including custom internal parts using Chakra components.
  • Create a custom theming config ReactSelect to be sent to the internal parts
  • Update implementation with related page components.
  • Update onChange handler functions to match new TypeScript support.

@netlify Netlify
Copy link

netlify bot commented Jan 30, 2024 •
edited
Loading

✅ Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit dcd8395
🔍 Latest deploy log https://app.netlify.com/sites/ethereumorg/deploys/65de05a3be166f00078fefed
😎 Deploy Preview https://deploy-preview-12065--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the dependencies 📦 Changes related to project dependencies label Jan 30, 2024
@TylerAPfledderer TylerAPfledderer changed the title build(deps): bump react-select to v5.8.0 feat: build custom react-select component with Chakra [updated] Jan 30, 2024
@github-actions github-actions bot added the tooling 🔧 Changes related to tooling of the project label Jan 30, 2024
@TylerAPfledderer TylerAPfledderer force-pushed the refactor/react-select-new-ds branch from fc9082f to dbdf622 Compare January 30, 2024 22:13
@TylerAPfledderer TylerAPfledderer marked this pull request as ready for review February 2, 2024 04:35
TylerAPfledderer
TylerAPfledderer commented Feb 3, 2024
View reviewed changes
src/components/ReactSelect/ReactSelect.stories.tsx Outdated

import ReactSelect, { ReactSelectOnChange } from "."

// TODO: Work with closed PR diff: https://github.com/ethereum/ethereum-org-website/pull/10542/files
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This storybook file is not complete. Should have a story displaying both style variants with the full logic combination including option groups and as combobox.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pettinarip along with your initial suggestions, I will ping you back when I have this story file finalized proper.

pettinarip
pettinarip reviewed Feb 12, 2024
View reviewed changes
Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

Wow! this looks great @TylerAPfledderer!!! ❤️

Few things I saw while testing:

  • Missaligned selects in the layer 2 page
    image

  • Group label color is black & white. Should be primary.highContrast
    image
    image

======

cc @nloureiro 👀

Pages where we are using the select:

@nloureiro
Copy link
Contributor

@TylerAPfledderer, great work, thank you!!! While testing on the browser, I realized that the color scheme was too strong and relied heavily on the primary color... Can you change it?

Sorry about this, but sometimes, when on the browser and in the mix of the styles, we can find "design bugs".
Screen Shot 2024-02-14 11 49 49 AM

Link to the Figma file (I added a comment, too) https://www.figma.com/file/NrNxGjBL0Yl1PrNrOT8G2B/ethereum.org-Design-System?type=design&node-id=5303%3A58208&mode=design&t=gfwqptwT1wv8ZJCx-1

Thank you!

@TylerAPfledderer
Copy link
Contributor Author

@pettinarip @nloureiro all above suggestions should be resolved, and have updated the storybook file.

@TylerAPfledderer
Copy link
Contributor Author

TylerAPfledderer commented Feb 15, 2024 •
edited
Loading

I'm afk at the time of this comment, but I noticed a styling error for the flush variant when checking the story book preview. Will get that addressed asap.

@nloureiro
Copy link
Contributor

I'm afk at the time of this comment, but I noticed a styling error for the flush variant when checking the story book preview. Will get that addressed asap.

not sure what you mean by "flush variant".... do you mean the active color?

I was checking and still found this difference, not sure if this is what you mean
Screen Shot 2024-02-15 09 38 11 AM

@TylerAPfledderer
Copy link
Contributor Author

I'm afk at the time of this comment, but I noticed a styling error for the flush variant when checking the story book preview. Will get that addressed asap.

not sure what you mean by "flush variant".... do you mean the active color?

I was checking and still found this difference, not sure if this is what you mean Screen Shot 2024-02-15 09 38 11 AM

I'm referring to the flushed versus outline variant you have labelled in the DS. I made an update to the border styling which caused the variants to look the same.

This should now be resolved for a proper review.

@TylerAPfledderer TylerAPfledderer force-pushed the refactor/react-select-new-ds branch from 6e37d6b to 6547732 Compare February 15, 2024 17:07
@TylerAPfledderer
Copy link
Contributor Author

@nloureiro you can immediately view this component through this published storybook preview

@nhsz
Copy link
Member

nhsz commented Mar 6, 2024

@TylerAPfledderer @pettinarip is this ready to be merged? I need to use a Select component and would be very handy, thanks!

@TylerAPfledderer
Copy link
Contributor Author

@TylerAPfledderer @pettinarip is this ready to be merged? I need to use a Select component and would be very handy, thanks!

@nhsz I think this is good to go for the most part. What's left is removing the original select and it's story

pettinarip
pettinarip approved these changes Mar 6, 2024
View reviewed changes
Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

🚀

@pettinarip pettinarip merged commit bda9262 into ethereum:dev Mar 6, 2024
10 checks passed
@TylerAPfledderer TylerAPfledderer deleted the refactor/react-select-new-ds branch March 6, 2024 14:44
This was referenced Mar 13, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pettinarip pettinarip pettinarip approved these changes

@nloureiro nloureiro nloureiro approved these changes

@wackerow wackerow Awaiting requested review from wackerow wackerow is a code owner

@corwintines corwintines Awaiting requested review from corwintines corwintines is a code owner

@minimalsm minimalsm Awaiting requested review from minimalsm minimalsm is a code owner

@nhsz nhsz Awaiting requested review from nhsz

Assignees
No one assigned
Labels
dependencies 📦 Changes related to project dependencies tooling 🔧 Changes related to tooling of the project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TylerAPfledderer @nloureiro @nhsz @pettinarip