Skip to content

Add Cancel button to OptionMenuList #205

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

Merged
merged 6 commits into from
Jan 28, 2020
Merged

Conversation

rhyza
Copy link
Contributor

@rhyza rhyza commented Jan 24, 2020

Resolves PDS-335

Affects multiselect menus for ButtonSelect and Select/FormSelect.

Screen Shot 2020-01-23 at 4 17 03 PM

Screen Shot 2020-01-23 at 4 17 24 PM

@rhyza rhyza requested a review from a team as a code owner January 24, 2020 00:18
Copy link
Contributor

@vine77 vine77 left a comment

Choose a reason for hiding this comment

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

The description of PDS-335 should have been written better, but the "Cancel" button was intended to only show up in the case that applyImmediately is false, i.e. "Apply" and "Cancel" will show up together, but if there is a "Done" button (when applyImmediately is true), then there is no need for a "Cancel" button (since it doesn't actually undo the selections).

Also, if there's an easy way to increase the min-width of the dropdown just a little bit, that might be helpful:

Screen Shot 2020-01-24 at 10 18 35 AM

@rhyza rhyza requested a review from vine77 January 24, 2020 19:42
@vine77 vine77 force-pushed the pds-335-multiselect-cancel-button branch from 448e83d to eabe3dc Compare January 28, 2020 20:58
Copy link
Contributor

@vine77 vine77 left a comment

Choose a reason for hiding this comment

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

Looks great!

giphy 2020-01-28 12_57_24

@vine77 vine77 merged commit e69d6a8 into master Jan 28, 2020
@vine77 vine77 deleted the pds-335-multiselect-cancel-button branch January 28, 2020 20:58
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.

2 participants