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

fix(react-dropdowns): add ...rest to allow additional props #1762

Merged
merged 1 commit into from
Jul 17, 2023
Merged

fix(react-dropdowns): add ...rest to allow additional props #1762

merged 1 commit into from
Jul 17, 2023

Conversation

ju-Skinner
Copy link
Collaborator

@ju-Skinner ju-Skinner commented Jul 13, 2023 •
edited
Loading

fixes: https://kajabi.atlassian.net/browse/DSS-433

Description

The SelectDropdown did not allow different properties to be used. This was in part of not accepting ...rest. So we couldn't use properties such as isPinned, customTrigger, to name a few. In order to fix this, I elected to add ...rest to the list of props to SelectDropdown, OptionsDropdown, and ToolbarDropdown. This should prevent any future issues with props missing.

Screenshots

Before After
image image

Testing in sage-lib

Testing in kajabi-products

  1. Check out this branch DSS-433/sage-react-dropdown-position-fix in KP
  2. Navigate to /admin/sites/1/contacts?
  3. Make your viewport narrow so that it would attempt to render the dropdown on the right side. It should technically be impossible to replicate the before picture.

@kajabi-bot
fix(react-dropdowns): add ...rest to allow additional props
6810313 
these props need to be to pass down to the base component Dropdown

fixes: https://kajabi.atlassian.net/browse/DSS-433
@ju-Skinner ju-Skinner changed the title fix(react-dropdowns): add ...rest to allow additional props fix(react-dropdowns): add ...rest to allow additional props Jul 13, 2023
@ju-Skinner ju-Skinner self-assigned this Jul 13, 2023
@ju-Skinner ju-Skinner requested a review from a team July 13, 2023 22:06
@ju-Skinner ju-Skinner marked this pull request as ready for review July 13, 2023 22:06
pixelflips
pixelflips approved these changes Jul 14, 2023
View reviewed changes
Copy link
Member

@pixelflips pixelflips left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for taking these on!!

@pixelflips pixelflips requested a review from a team July 14, 2023 16:24
QuintonJason
QuintonJason approved these changes Jul 14, 2023
View reviewed changes
Copy link
Contributor

@QuintonJason QuintonJason left a comment

Choose a reason for hiding this comment

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

Looks and works great!

anechol
anechol approved these changes Jul 14, 2023
View reviewed changes
Copy link
Contributor

@anechol anechol left a comment

Choose a reason for hiding this comment

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

LGTM

@ju-Skinner ju-Skinner merged commit 094f754 into develop Jul 17, 2023
@ju-Skinner ju-Skinner deleted the fix/selectdropdown-missing-properties branch July 17, 2023 13:37
@ju-Skinner ju-Skinner mentioned this pull request Jul 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anechol anechol anechol approved these changes

@QuintonJason QuintonJason QuintonJason approved these changes

@pixelflips pixelflips pixelflips approved these changes

Assignees

@ju-Skinner ju-Skinner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ju-Skinner @pixelflips @QuintonJason @anechol @kajabi-bot