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: set flyout visbility on calling setExpanded #7199

Merged
merged 2 commits into from
Jul 5, 2023

Conversation

itsjayway
Copy link
Contributor

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Fixes #7174

Proposed Changes

call setVisible(isExpanded) on the parentToolbox_'s flyout to update visibility

Behavior Before Change

flyout is not closed when setExpanded(false) is called on collapsible_category, contrary to expected behavior

2023-06-23.01-51-08.mp4

Behavior After Change

flyout is hidden along with collapsed category

2023-06-23.01-44-01.mp4

Reason for Changes

fulfilling expected behavior in every case

Test Coverage

tested full functionality of expand/collapse by opening, switching, and unfocus clicking the flyout/collapsible regions before and after changes

Documentation

updated setExpanded` docstring

@itsjayway itsjayway requested a review from a team as a code owner June 23, 2023 05:55
@itsjayway itsjayway requested a review from BeksOmega June 23, 2023 05:55
@github-actions github-actions bot added the PR: fix Fixes a bug label Jun 23, 2023
@itsjayway
Copy link
Contributor Author

please let me know of any redundancies that may arise with setting the flyout's display attribute. I'm going to double check on my end as well. thank you in advance!

Copy link
Collaborator

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

Thank you for the fix! This is looking good :D Just one change and then this should be good to go!

core/toolbox/collapsible_category.ts Outdated Show resolved Hide resolved
@itsjayway itsjayway requested a review from BeksOmega July 4, 2023 15:54
@BeksOmega BeksOmega merged commit 57fdb71 into google:develop Jul 5, 2023
@itsjayway itsjayway deleted the closeFlyoutOnSetExpandedFalse branch July 5, 2023 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
2 participants