Skip to content

Pf vertical navigation ui sref support #345

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Nov 18, 2016
Merged

Pf vertical navigation ui sref support #345

merged 8 commits into from
Nov 18, 2016

Conversation

lorthirk
Copy link
Contributor

Implements #342. Credits to @MattiaDalBen for the unit tests and the help for the optional service injection.

@dtaylor113
Copy link
Member

LGTM. cloned this branch. all unit tests passed and ngdocs Navigation example works fine with these changes.
@lorthirk It would be great if you could update the pf-vert-nav ngdoc example to indicate support for angular-ui-router. Please include any setup or usage examples required to use pf-vert-nav and angular-ui-router. -thanks!

@lorthirk
Copy link
Contributor Author

@MattiaDalBen it would be great if you could cover this tomorrow

@mattdibi
Copy link
Contributor

@lorthirk I'm on it!

@mattdibi
Copy link
Contributor

@dtaylor113 as requested I updated the pf-vert-nav ngdoc example.

@dtaylor113
Copy link
Member

Hi @MattiaDalBen, thanks it looks great. One small nit, could you please add an initial topic sentence to the beginning of the 'pfVerticalNavigation - Router' ngdoc example so that it differentiates itself from the 'Basic' example. Something like: "This example shows how to use pfVerticalNavigation with angular-ui-router's $states and uiSrefs." -thanks!

jeff-phillips-18
jeff-phillips-18 approved these changes Nov 18, 2016
View reviewed changes
Copy link
Member

@jeff-phillips-18 jeff-phillips-18 left a comment

Choose a reason for hiding this comment

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

👍

@mattdibi
Copy link
Contributor

@dtaylor113 Updated description.

@dtaylor113
Copy link
Member

LGTM! Thanks

@dtaylor113 dtaylor113 merged commit d99a10b into patternfly:master Nov 18, 2016
@lorthirk lorthirk deleted the pfVerticalNavigation-uiSref-support branch November 18, 2016 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeff-phillips-18 jeff-phillips-18 jeff-phillips-18 approved these changes

+1 more reviewer

@dtaylor113 dtaylor113 dtaylor113 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lorthirk @dtaylor113 @mattdibi @jeff-phillips-18