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

feat: add SelectWithIcon component #9

Closed
wants to merge 18 commits into from
Closed

Conversation

vickyyyyyyy
Copy link
Member

Select component including buttons to add and remove the field.
This can be used as an alternative to the -- remove -- option that currently exists in our dropdowns.

displayIcon
Toggle for displaying the add button

isMulti
Supports both Select and MultiSelect

children
Renders elements between the Select field and the remove button

renderRemove
Toggle for displaying the remove button

onRemove
Function bound to remove button

addIcon
Overrides the default icon for adding

removeIcon
Overrides the default icon for removing

onClickOutside
Function bound to clicks outside of the Select

Work done:

  • Added component

  • Added test fixtures

  • Added lodash to use kebabCase for the option test fixtures

  • Added @testing-library/user-event for better event handling and it is also suggested by RTL

  • Added @babel/plugin-transform-runtime to fix following error when trying to run async tests:
    Screenshot 2020-12-07 at 17 51 10

  • Bumped react-dom down to match current react version and also, it looks like react-dom version 17+ causes the tests to fail when there are components wrapped within ClickOutsideWrapper

Basic:
basic select

Multi:
multi select

Remove Option:
remove select

Children:
children select

Custom Add Icon:
custom add icon select

Custom Remove Icon:
custom remove icon select

@vickyyyyyyy vickyyyyyyy requested a review from a team December 7, 2020 18:42
Copy link
Contributor

@scottlepp scottlepp left a comment

Choose a reason for hiding this comment

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

Is there an issue this component resolves? This is needed in some plugin?

Probably want someone from ux to review this.

@vickyyyyyyy
Copy link
Member Author

Is there an issue this component resolves? This is needed in some plugin?

Just a suggestion for a better way of adding and removing fields - at the moment, across multiple plugins, we are removing fields by relying on selecting a specific -- remove -- option, which isn't really how the dropdown options should be used (and relies on pushing a specific remove option right to the top of the options array). If we had a remove button instead, this should help with the UX and tidy up the code.

Example of removing via the dropdown options:
Screenshot 2020-12-08 at 17 15 37

Probably want someone from ux to review this.

Sounds good, asked UX for review here: https://raintank-corp.slack.com/archives/G014QJ45DD4/p1607365776010900

@vickyyyyyyy
Copy link
Member Author

closing this - the component has changed considerably in the SNOW plugin from this. If we want to revisit and extract it out, we can open another PR with the updated changes

@vickyyyyyyy vickyyyyyyy closed this Jan 7, 2021
@vickyyyyyyy vickyyyyyyy deleted the selectWithIcon branch January 22, 2021 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants