pfVerticalNavigation: on mobile, move the masthead utility items to the vertical navigation #747
Conversation
There was a problem hiding this comment.
Might want to address #746 while you are working here. It should effect the hide/show of the nav header items in the navigation menu as well.
| iconClass: "fa pficon-help", | ||
| href: "#/help", | ||
| mobileOnly: "true" | ||
| }, |
There was a problem hiding this comment.
This should have Help and About as children to be consistent with the header.
There was a problem hiding this comment.
These changes should be made in the -router example as well
There was a problem hiding this comment.
This should have Help and About as children to be consistent with the header.
Wasn't crazy about having sub-menus:
Help -> Help
About
So left out 'About' as maybe something you wouldn't need at mobile.
There was a problem hiding this comment.
OK, just thinking we should show consistency.
| @@ -30,7 +30,7 @@ | |||
| 'force-hide-secondary-nav-pf': $ctrl.forceHidden, | |||
| 'show-mobile-nav': $ctrl.showMobileNav}"> | |||
| <ul class="list-group"> | |||
| <li ng-repeat="item in $ctrl.items" class="list-group-item" | |||
| <li ng-repeat="item in $ctrl.items | navItems:$ctrl.inMobileState" class="list-group-item" | |||
There was a problem hiding this comment.
Might be easer just to add:
'visible-xs': item.mobileOnly
to the ng-class rather than filtering.
| iconClass: "fa pficon-user", | ||
| mobileOnly: "true", | ||
| children: [ | ||
| { title: "Preferences" }, |
There was a problem hiding this comment.
Can we change Preferences to User Preferences both here and in the header area? It was a UX request in the patternfly-react repo, so might as well update here as well.
| @@ -35,6 +35,8 @@ | |||
| * <li>.tooltip - (string) Tooltip to display for the badge | |||
| * <li>.badgeClass: - (string) Additional class(es) to add to the badge container | |||
| * </ul> | |||
| * <li>.mobileOnly - (string) When set to 'true', menu item will only be displayed when browser is in mobile mode (<768px). | |||
There was a problem hiding this comment.
why use a string here rather than a boolean?
dtaylor113
force-pushed
the
vert-nav-items-mobile-only
branch
from
0db48ea to
f15ae12
Compare
|
Hi @jeff-phillips-18, I believe I have addressed your comments, except the one about the Help-at-mobile menu, IMO I think leaving off the 'About' menu item represents a truer @mobile menu. |
I'd prefer to do that in a follow up pr, thanks :-) |
…ects; when 'true' items only display in mobile mode.
dtaylor113
force-pushed
the
vert-nav-items-mobile-only
branch
from
f15ae12 to
fb4ffeb
Compare
Description
Added a
mobileOnlyproperty to nav. item objects; when 'true' items only display in mobile mode.Addresses: #745
Ex: The Help and User vert. nav items only appear when in mobile (< 768px) mode, and are hidden when not in mobile mode:
Note the masthead utility items (Help & User dropdowns) continue to be hidden at mobile; this PR allows app devs to mimic them 'moving' to the vert nav menu.
@alexkieling
PR Checklist