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 logic error that prevented multi-select dropdowns from closing properly #3652

Merged
merged 1 commit into from
May 13, 2020

Conversation

tylercloke
Copy link
Contributor

🔩 Description: What code changed, and why?

toggleDropdown was calling closeDropdown, which set active to false, but then toggleDropdown flipped the value of active for both the open and close case, incorrectly setting active back to true after closeDropdown was called.

👟 How to Build and Test the Change

Create a project then go to a multi-select dropdown (say in teams create modal).

Click on the dropdown and it will open.

Now, do any of the following and it should close properly:

  • click outside the dropdown.
  • click the dropdown header again.
  • press enter.

✅ Checklist

@susanev susanev added auth-team anything that needs to be on the auth team board automate-ui bug 🐛 Something isn't working ui labels May 12, 2020
Copy link
Contributor

@msorens msorens left a comment

Choose a reason for hiding this comment

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

🎉 Just one suggestion.

@@ -74,11 +74,10 @@ export class ResourceDropdownComponent implements OnInit, OnChanges {
if (!this.active) { // opening
this.filterValue = '';
this.filteredResources = this.resourcesInOrder;
this.active = !this.active;
Copy link
Contributor

Choose a reason for hiding this comment

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

Because this is now used only in a single case, I think it improves clarity to do this:

Suggested change
this.active = !this.active;
this.active = true;

(Just like there is this.active = false; on L95.)

@susanev
Copy link
Contributor

susanev commented May 13, 2020

it seems that #3533 is not fixed with this, so there may still be a click outside functionality issue that can be solved in that other issue. but toggle works great now, thank you!!

…operly

toggleDropdown was calling closeDropdown, which set active to false, but then toggleDropdown flipped the value of active for both the open and close case, incorretly setting active back to true after closeDropdown was called.

Signed-off-by: Tyler Cloke <tylercloke@gmail.com>
@tylercloke tylercloke merged commit 0278ed9 into master May 13, 2020
@chef-expeditor chef-expeditor bot deleted the tc/dropdown-fail branch May 13, 2020 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth-team anything that needs to be on the auth team board automate-ui bug 🐛 Something isn't working ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants