patternfly / patternfly-design

Use this repo to file all new feature or design change requests for the PatternFly project
114 stars 104 forks source link

Restyle tertiary navigation #876

Closed ajacobs21e closed 3 years ago

ajacobs21e commented 4 years ago

Requesting secondary horizontal navigation.

The RH web site systems use a horizontal 'megamenu' style navigation which is similar to PF4's horizontal navigation. Some sites have secondary navigation with links to other pages. I don't see a PF4 horizontal secondary navigation.

Ideally this would not be confused with horizontal jump links (in page nav) https://github.com/patternfly/patternfly-design/issues/875

mcarrano commented 4 years ago

@ajacobs21e we do have a secondary horizontal nav that we call "tertiary" navigation: https://www.patternfly.org/v4/documentation/react/components/nav#tertiary Would that work or is this really a new menu? Do you have any mocks that show what this might look like?

ajacobs21e commented 4 years ago

@mcarrano This is a little out of date but something like this is what we need (the grey bar) image

@coreyvickery @starryeyez024 I think the tertiary nav meets our needs. Are there any other requirements that aren't met by this component?

coreyvickery commented 4 years ago

@ajacobs21e Not 100% sure since we don't use this navigation on redhat.com.

@dcaryll Do we need to spec anything else for Allie? If not, I think this looks good.

dcaryll commented 4 years ago

@ajacobs21e @coreyvickery here are some instances where we need to solve for secondary/tertiary nav. Not sure if horizontal always works.

maryshak1996 commented 4 years ago

@ajacobs21e not sure if you've seen any of this yet, but @mindreeper2420 has been working on some explorational design work around a trending nav, and I'll be weighing in on the design side too. Do you want to be looped into those conversations?

ajacobs21e commented 4 years ago

@maryshak1996 yes please! It's not exactly the same as the Portal subnav but I am hoping we can align the two as much as possible

maryshak1996 commented 4 years ago

@ajacobs21e cool! I'll keep you in the loop :) adam has been sharing his work with us here, if you wanna check it out: https://redhat-developer.github.io/design-manual/components/topic-nav

ajacobs21e commented 3 years ago

For the web use case, we'll use the tertiary nav for a secondary horizontal nav https://www.patternfly.org/v4/components/page/react-demos#tertiary-nav

and the default nav for a side nav https://www.patternfly.org/v4/components/page/react-demos#expandable-nav Corey's design https://xd.adobe.com/view/770b8c85-43d8-4dd5-7d1f-83ed13b27d77-013a/

ajacobs21e commented 3 years ago

@mceledonia @lboehling When using the RCUE red for a highlight, I think it looks bad with blue text image

Here are two options we could do 1) Keep all text black and just use the red highlight to indicate selected item & hover state black This is similar to how the web site system navigation works for a direct link. The text color stays the same and a red highlight is used for hover and click https://www.sketch.com/s/ffaeec24-7590-4017-a574-52b8938b4de8/a/EAjLWd

2) Black text for active, grey (color-200) for inactive grey This matches what we do for tabs RCUE tabs

Which do you think works best? I think we should try to define this and globally as possible for all RCUE components if you also agree link blue + brand red = ew

@dcaryll @coreyvickery

coreyvickery commented 3 years ago

@ajacobs21e Definitely more of a fan of #2.

mcarrano commented 3 years ago

Per discussion with @mceledonia @mcoker @ajacobs21e @coreyvickery and @gdoyle1 we decided that this issue is best satisfied by restyling the existing tertiary nav component to be more usable as a secondary horizontal nav for web and other related use cases. Specifically, want to:

Am I missing anything?

gdoyle1 commented 3 years ago

I started on this here: Marvel project | Marvel prototype

One of the major concerns I found with this is that the wording "tertiary navigation" is confusing. A lot of the time that this component is used, the horizontal navigation is used (within the masthead) as primary and then tertiary is used as "secondary". I tried coming up with other ways to approach secondary and tertiary from a horizontal navigation standpoint (as we see with redhat.com and other websites)

We have that meeting to review next week, so no rush in commenting before then! @mcarrano @mceledonia

mcarrano commented 3 years ago

Thanks @gdoyle1. Agree that the name "tertiary" nav is unfortunate, so maybe we would change that or just re-introduce this as Seondary navigation.

gdoyle1 commented 3 years ago

Updated the Marvel based on our discussion @mcarrano @mceledonia https://marvelapp.com/prototype/fj0941h - @coreyvickery I tried adding a version of it that was sticky and had a shadow - like we discussed yesterday, you can obviously change the colors and such, but I wanted to try it out!

mcarrano commented 3 years ago

This is looking good @gdoyle1 . I just have a couple of questions.

Did we agree to try creating space between the masthead and secondary nav when used with a horizontal primary? That was one way to get around the problem of the blue primary item sitting directly on top of the secondary bar. Or should we just not concern ourselves with the since it's not a configuration we recommend for product?

That's for creating the example combining this with the vertical as a tertiary nav. I really like it, but we should plan to review with other product stakeholders as a recommendation to move away from the tab style tertiary nav. One nit about this however. It feels like there should be a border on the left side of the first item to separate it from the vertical. See below. Looks like the background color of the two flows across. What do others think?

Screen Shot 2021-02-11 at 3 01 41 PM

gdoyle1 commented 3 years ago

@mcarrano @mceledonia Here are some options for space between the horizontal primary nav and the sub nav! I also included an outline like you asked about above in one of the versions, Matt

sub nav examples

mcarrano commented 3 years ago

I definitely prefer V1 @gdoyle1 as in the second version I still feel like the blue line is associated with the menu item below.

@mcoker if we want to change the offset of the blue select underline highlight on our primary horizontal nav, would you see that as a breaking change?

mcoker commented 3 years ago

if we want to change the offset of the blue select underline highlight on our primary horizontal nav, would you see that as a breaking change?

@mcarrano I think it would be fine. My only concern is changing anything that is part of the page chrome as it may seem unexpected, though it's a very small change and I wouldn't expect suspect it to have much/any impact.

gdoyle1 commented 3 years ago

@mcarrano @mcoker I also tried extending the line a bit so it would be 16px longer than the nav item itself. I don't think that's a MUST have if we were to make this change though - was trying it out after @mceledonia 's suggestion!

gdoyle1 commented 3 years ago

Hey all @mcarrano @mceledonia @coreyvickery - I tried adding another menu on the left for the sub nav. I don't think we should go with that - it looks strange having two hamburgers stacked on top of each other. If we did this, I think we'd have to add a third level item (tertiary) to the left navigation in order to connect the groups.

Screen Shot 2021-03-10 at 2 34 52 PM

(Recommended mobile version all the way to the right)

mcarrano commented 3 years ago

I totally agree @gdoyle1 . So one solution (and maybe the only solution) is horizontal scrolling, which is what happens with our tertiary nav now. The other option would be to somehow combine the two menus into the vertical panel. So if you select a page with an additional level, you can drill down into it from from within the single vertical menu. I believe that's what you proposed @coreyvickery if I'm not mistaken. I'm just not sure how that works from a responsive HTML/CSS standpoint since it implies two menus (vertical + horizontal) would morph into one (vertical with hamburger). @mcoker any thoughts here? Is there a way to make that work?

mcoker commented 3 years ago

I'm just not sure how that works from a responsive HTML/CSS standpoint since it implies two menus (vertical + horizontal) would morph into one (vertical with hamburger)

@mcarrano the react component could watch the window size, and at a specified size, move the tertiary nav into the sidebar. Though I think we want to avoid react from having to watch the window size as I believe that can hinder page performance.

Another way, and maybe this is preferred, is the react component could copy the tertiary nav into the sidebar on page load if the user has specified, and the CSS could manage hiding the tertiary nav in the masthead on mobile, and show the nav in the sidebar instead.

Another way could be to have the user duplicate their tertiary nav as they would like it to appear in the sidebar (they could change items, use icons, etc), and the CSS manages hiding/showing it at a specified breakpoint. I believe this is how the PF3 navigation worked - users replicated masthead dropdown items in the vertical navigation, and we showed them in the vertical nav and hid them in the masthead at a specified breakpoint. It puts more responsibility on the user to duplicate the code, but gives them a little more flexibility in how it appears in the sidebar.

gdoyle1 commented 3 years ago

@mcarrano @mceledonia @mcoker We decided on just using the arrow overflow method, which matches what we're doing with horizontal nav today, but will open a follow-up issue to look more into the methods you described above.

Here's the marvel for edits to existing horizontal nav and a spec for the new subnav https://marvelapp.com/prototype/fj0941h. Please contact me if you need anything!

mcarrano commented 3 years ago

Thanks @gdoyle1 ! I have opened two new core issues to track implementation:

https://github.com/patternfly/patternfly/issues/3935 - to add the new horizontal sub-nav variant. https://github.com/patternfly/patternfly/issues/3936 - to update the tertiary nav demo to use this variant in a page.

mcarrano commented 3 years ago

Design work is complete, so will close this.