Skip to content

Conversation

lnewson
Copy link
Contributor

@lnewson lnewson commented Mar 16, 2018

Fixes #728

Description

Fixes an issue with the vertical navigation, where the classes won't be updated when resizing based on the ng-class definition. This was causing the navigation to remain shown when resizing to a smaller window and then remaining hidden when resizing back to a larger screen, until some event in the application caused a $digest to occur. Upon investigating the issue, this is caused by the resize event handler being processed outside of AngularJS's context and therefore won't trigger any watchers and won't update the classes based on the ng-class definition.

Note: This is actually a regression between 3.x -> 4.x, though I've fixed this in a slightly different way to 3.x, so that this only triggers a digest on the isolated component scope, instead of the root scope. This approach means it should be better for performance, as it only needs to evaluate the watchers in the component scope, instead of evaluating all watchers.

PR Checklist

  • Unit tests are included
  • Screenshots are attached (if there are visual changes in the UI)
  • A Designer (@beanh66) is assigned as a reviewer (if there are visual changes in the UI)
  • A CSS rep (@cshinn) is assigned as a reviewer (if there are visual changes in the UI)

Copy link
Member

@dtaylor113 dtaylor113 left a comment

Choose a reason for hiding this comment

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

Hi @lnewson, downloaded and ran this PR branch, everything looks good and works the way you have described. Thank you for this contribution and for being so thorough in your research in addressing this issue!

@dtaylor113 dtaylor113 merged commit b2ac31c into patternfly:master Mar 16, 2018
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.

pfVerticalNavigation doesn't hide/show correctly when resizing
2 participants