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

fix(insert-variables): Dropdown shouldn't store the option selected. #468

Merged
merged 10 commits into from
Mar 17, 2022

Conversation

creador-dev
Copy link
Member

No description provided.

@creador-dev creador-dev added the improvement Improvements for current features label Feb 28, 2022
@creador-dev creador-dev changed the title enhance(insert variables): removed selected value in insert variables ✨ new(insert variables): removed selected value in insert variables Feb 28, 2022
@iamleigh iamleigh changed the title ✨ new(insert variables): removed selected value in insert variables new(insert-variables): Dropdown shouldn't store the option selected. Mar 4, 2022
@iamleigh iamleigh changed the title new(insert-variables): Dropdown shouldn't store the option selected. fix(insert-variables): Dropdown shouldn't store the option selected. Mar 4, 2022
Copy link
Contributor

@iamleigh iamleigh left a comment

Choose a reason for hiding this comment

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

@creador-dev review again your changes, please. Remember, if developers use the SUI.select.initVars function they need to get everything from it. There's no need to re-add the clear function again because if it doesn't work right now on the showcase it won't for sure on the plugins. Dig a bit more and try a better solution.

@creador-dev creador-dev changed the title fix(insert-variables): Dropdown shouldn't store the option selected. fix(insert-variables): dropdown shouldn't store the option selected. Mar 4, 2022
@creador-dev creador-dev changed the title fix(insert-variables): dropdown shouldn't store the option selected. 🐛 fix(insert-variables): dropdown shouldn't store the option selected. Mar 4, 2022
@creador-dev creador-dev requested a review from iamleigh March 7, 2022 05:20
page-insert-variables.html Outdated Show resolved Hide resolved
assets/js/showcase/pages/insert-variables.js Outdated Show resolved Hide resolved
@iamleigh iamleigh changed the title 🐛 fix(insert-variables): dropdown shouldn't store the option selected. fix(insert-variables): Dropdown shouldn't store the option selected. Mar 13, 2022

let select = $( this );

select.val( null ).trigger( 'change' );
Copy link
Contributor

Choose a reason for hiding this comment

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

@creador-dev why not try clearing the selected value when the dropdown closes? You will simplify the code, and also we make sure to have the best experience for screen reader users.

@creador-dev creador-dev requested a review from iamleigh March 14, 2022 13:11
@iamleigh iamleigh merged commit d262835 into version/2.12.5 Mar 17, 2022
@iamleigh iamleigh deleted the new/SUI-294 branch March 17, 2022 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvements for current features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants