Carbon 10 navbar & menu #6963

himdel · 2020-04-08T13:30:30Z

Related PRs (all dependencies merged now):

(cosmetic, reorder menus) Menu - sort Providers menu together, singular #6994
(optional; drop big_iframe remnants) Remove obsolete big_iframe logic #7017
(optional; replace sass implementation) sass - switch to dart sass #7018
(just add carbon) Carbon - add carbon scss #7068
(menu cleanups from before) Menu - pre-carbon cleanups #7069
(overridable components, ids) React - override id, component; convert Breadcrumbs to functional #7070
(common code to generate href, onClick & target for menu items) Menu link helper #7071
(optional; fix webpack warning) autoprefixer, browserslist: update, move to package.json #7072
(optional; drop menu settings loader) Menu - drop SettingsLoader #7078
(cleanup after; drop "remember last item in section" feature) Remove maintab, rembember_tab, session[:tab_url] functionality #7088

This PR replaces our menu & navbar with a carbon menu design (#6986), without a navbar,
switches menu (and notification button) to carbon icons,
moves the notification button to the breadcrumb area,
merges the help menu section, user settings and server configuration under one Settings menu,
adds 2 new (non-customizable) logos: manageiq-logo-inverse.svg & manageiq-logo-glyph-inverse.svg (collapsed) for the menu (and minifies brand.svg),
moves all menu, navbar & notifications styling to webpack (and removes obsolete entries),
and makes react router router changes (currently only used by v2v) work out of the box by watching for route changes.

Registered components:

menu.MainMenu - the menu
menu.Navbar - an empty navbar stub

Relevant Carbon docs:

menu - UIShell
react - storybook
theme - Gray 90 is the menu theme

himdel · 2020-04-09T04:43:39Z

Update:

himdel · 2020-05-01T18:22:17Z

Current state..

EDIT: sizing and new icons

himdel · 2020-05-04T19:55:31Z

2nd level SideNav almost ready..

himdel · 2020-05-22T13:48:02Z

Menu collapsed:

Menu default, with group switcher:

Menu default, with 1 group:

Menu search results:

Secondary and third-level menu:

himdel · 2020-05-29T12:12:49Z

Cc @skateman

As far as I know, the only thing I need to fix is ~~wrap the breadcrumb spec in redux, and fix~~ v2v active items, otherwise this is ready.

So, feel free to review :)

skateman

I have two major readability concerns:

mixing too much conditional logic into the body of the return()
forcing multiline/indented markup for one/two props on a single element

skateman · 2020-05-30T11:42:56Z

app/javascript/components/breadcrumbs/index.jsx

@@ -45,14 +46,17 @@ const renderItems = ({ items, controllerName }) => {
 };

 const Breadcrumbs = ({ items, title, controllerName }) => (


I woudln't call it just Breadcrumbs if we also put the notifications into it.

left Breadcrumbs toggle-less and added BreadcrumbsBar to wrap both 👍

skateman · 2020-05-30T11:44:58Z

app/javascript/components/breadcrumbs/notifications-toggle.jsx

+  return (
+    <a
+      id="notifications-toggle"
+      className={`btn btn-default ${isDrawerVisible && 'active'} ${unreadCount && 'unread'}`}


You can use classNames here.

skateman · 2020-05-30T11:45:09Z

app/javascript/components/breadcrumbs/notifications-toggle.jsx

+      onClick={toggle}
+    >
+      {__("Notifications")}
+      &nbsp;


looks like {' '} is more standard, updating

skateman · 2020-05-30T11:45:34Z

app/javascript/menu/README.md

@@ -0,0 +1,33 @@
+## ManageIQ Main Menu & Navbar


Shouldn't be this part of guides instead?

I'm hoping the closer to the code this is, the bigger the chance people will maintain it
¯\_(ツ)_/¯

I'm just worried about fragmentation of our docs.

skateman · 2020-05-30T11:46:33Z

app/javascript/menu/group-switcher.jsx

+const { miqChangeGroup } = window;
+
+export const GroupSwitcher = ({ miqGroups, currentGroup, expanded }) => {
+  const options = miqGroups.map((g) => ({


What about destructuring, then you don't need the g and this can be a one-liner.

skateman · 2020-05-30T11:50:34Z

app/javascript/menu/main-menu.jsx

+
+          <hr className="bx--side-nav__hr" />
+
+          {searchResults && <SearchResults


Same here, a one-liner is more readable.

app/javascript/menu/main-menu.jsx

skateman · 2020-05-30T11:51:28Z

app/javascript/menu/search-results.jsx

+    <p className="menu-search-title">
+      {titles[titles.length - 1]}
+    </p>
+    <p className="menu-search-parent">


This is super hard to follow below 😕

skateman · 2020-05-30T11:51:36Z

app/javascript/menu/search-results.jsx

+const Count = ({ length }) => (
+  <p>
+    {__("Results")}
+    &nbsp;


skateman · 2020-05-30T11:53:12Z

app/javascript/menu/second-level.jsx

+import { itemId, linkProps } from './item-type';
+
+const mapItems = (items) => items.map((item) => (
+  item.items.length


What about pulling out the component selection logic outside from the render?

const Component = item.items.length ? MenuSection : MenuItem; return <Component key={item.id} {...item} />

first level section/item take different props

EDIT: simplified

himdel · 2020-06-02T20:49:35Z

Added support for hiding secondary menu after clicking a menu item (about modal & react routes don't do full page reload),
and miqSparkleOn, followed by miqSparkleOff on the first react router history change

basically v2v support, but it will work for any plugin which uses `.menu` and `HashRouter` would need different, simpler `currentUrl` logic for `BrowserRouter`

himdel · 2020-06-04T23:16:44Z

Ok, this also supports v2v menu items now by watching for history changes,
so I think this is ready ready :). Or ready for another round of bashing anyway :).

ManageIQ/manageiq-v2v#1136 removes the obsolete v2v code.

himdel · 2020-06-04T23:16:58Z

@miq-bot remove_label wip

miq-bot · 2020-06-04T23:21:04Z

skateman

skateman · 2020-06-05T10:22:01Z

LGTM 👍 @h-kataria can you please also click through this a little bit?

h-kataria · 2020-06-06T18:24:05Z

Tried in UI, Looks good.

Clicking top level menu items is no longer supported with the Carbon styles, so fixing the specs to better reflect the current functionality, or commenting out places where it isn't valid. Most likely broken because of these changes: ManageIQ#6963

NOTE: Pulling this in from another branch, but is already part of a separate PR. Just doing to help make CI "less red" Clicking top level menu items is no longer supported with the Carbon styles, so fixing the specs to better reflect the current functionality, or commenting out places where it isn't valid. Most likely broken because of these changes: ManageIQ#6963

Clicking top level menu items is no longer supported with the Carbon styles, so fixing the specs to better reflect the current functionality, or commenting out places where it isn't valid. Most likely broken because of these changes: ManageIQ#6963

NOTE: Pulling this in from another branch, but is already part of a separate PR. Just doing to help make CI "less red" Clicking top level menu items is no longer supported with the Carbon styles, so fixing the specs to better reflect the current functionality, or commenting out places where it isn't valid. Most likely broken because of these changes: ManageIQ#6963

himdel added the wip label Apr 8, 2020

miq-bot added the unmergeable label Apr 8, 2020

miq-bot removed the unmergeable label Apr 9, 2020

himdel linked an issue Apr 14, 2020 that may be closed by this pull request

Apply Carbon Styling to header, navbars and menus #6817

Closed

himdel removed a link to an issue Apr 14, 2020

Apply Carbon Styling to header, navbars and menus #6817

Closed

chessbyte linked an issue Apr 14, 2020 that may be closed by this pull request

Apply Carbon Styling to header, navbars and menus #6817

Closed

Fryguy mentioned this pull request Apr 20, 2020

ManageIQ Navigation design #6986

Closed

miq-bot added graphics unmergeable labels May 7, 2020

miq-bot removed the unmergeable label May 18, 2020

This was referenced May 21, 2020

Carbon - add carbon scss #7068

Merged

Menu - pre-carbon cleanups #7069

Merged

React - override id, component; convert Breadcrumbs to functional #7070

Merged

Menu link helper #7071

Merged

miq-bot added the unmergeable label May 22, 2020

himdel added the jansa/yes? label May 22, 2020

miq-bot removed the unmergeable label May 26, 2020

himdel mentioned this pull request May 27, 2020

Menu - drop SettingsLoader #7078

Merged

ZitaNemeckova mentioned this pull request May 27, 2020

Request to add quick search filter on the User's "change group" menu. #7065

Open

miq-bot added the unmergeable label May 28, 2020

himdel mentioned this pull request May 28, 2020

Remove maintab, rembember_tab, session[:tab_url] functionality #7088

Merged

miq-bot removed the unmergeable label May 29, 2020

himdel changed the title ~~[WIP] [POC] Carbon 10 navbar & menu~~ [WIP] Carbon 10 navbar & menu May 29, 2020

skateman suggested changes May 30, 2020

View reviewed changes

Menu - add history support for react router

e2aab96

basically v2v support, but it will work for any plugin which uses `.menu` and `HashRouter` would need different, simpler `currentUrl` logic for `BrowserRouter`

miq-bot changed the title ~~[WIP] Carbon 10 navbar & menu~~ Carbon 10 navbar & menu Jun 4, 2020

miq-bot added dependencies and removed wip labels Jun 4, 2020

skateman approved these changes Jun 5, 2020

View reviewed changes

skateman assigned h-kataria Jun 5, 2020

h-kataria approved these changes Jun 6, 2020

View reviewed changes

h-kataria merged commit 3eba098 into ManageIQ:master Jun 6, 2020

himdel deleted the carbon10 branch June 7, 2020 21:10

h-kataria added the enhancement label Jun 9, 2020

himdel mentioned this pull request Jun 11, 2020

Vertical Nav responsive state updates #5907

Closed

chessbyte added jansa/no and removed jansa/yes? labels Jun 11, 2020

himdel mentioned this pull request Jun 18, 2020

Remove old navbar styling from brand.scss #7143

Merged

himdel mentioned this pull request Jul 7, 2020

Ivanchuk navbar - broken coloring #6983

Closed

NickLaMuro mentioned this pull request Jul 10, 2020

[search_box.spec.js] Fix failing specs #7191

Merged

NickLaMuro mentioned this pull request Jul 10, 2020

Fix menu.spec.js #7192

Merged

h-kataria mentioned this pull request Jul 15, 2020

Fixed to translate menu titles in Navbar #7204

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Carbon 10 navbar & menu #6963

Carbon 10 navbar & menu #6963

himdel commented Apr 8, 2020 •

edited

Loading

himdel commented Apr 9, 2020 •

edited

Loading

himdel commented May 1, 2020 •

edited

Loading

himdel commented May 4, 2020

himdel commented May 22, 2020

himdel commented May 29, 2020 •

edited

Loading

skateman left a comment

skateman May 30, 2020

himdel Jun 2, 2020

skateman May 30, 2020

skateman May 30, 2020

himdel Jun 2, 2020

skateman May 30, 2020

himdel Jun 2, 2020

skateman Jun 5, 2020

skateman May 30, 2020

skateman May 30, 2020

skateman May 30, 2020

skateman May 30, 2020

skateman May 30, 2020

himdel Jun 2, 2020 •

edited

Loading

himdel commented Jun 2, 2020

himdel commented Jun 4, 2020

himdel commented Jun 4, 2020

miq-bot commented Jun 4, 2020

skateman left a comment

skateman commented Jun 5, 2020

h-kataria commented Jun 6, 2020

		@@ -45,14 +46,17 @@ const renderItems = ({ items, controllerName }) => {
		};

		const Breadcrumbs = ({ items, title, controllerName }) => (


		<hr className="bx--side-nav__hr" />

		{searchResults && <SearchResults

Carbon 10 navbar & menu #6963

Carbon 10 navbar & menu #6963

Conversation

himdel commented Apr 8, 2020 • edited Loading

himdel commented Apr 9, 2020 • edited Loading

himdel commented May 1, 2020 • edited Loading

himdel commented May 4, 2020

himdel commented May 22, 2020

himdel commented May 29, 2020 • edited Loading

skateman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himdel Jun 2, 2020 • edited Loading

Choose a reason for hiding this comment

himdel commented Jun 2, 2020

himdel commented Jun 4, 2020

himdel commented Jun 4, 2020

miq-bot commented Jun 4, 2020

skateman left a comment

Choose a reason for hiding this comment

skateman commented Jun 5, 2020

h-kataria commented Jun 6, 2020

himdel commented Apr 8, 2020 •

edited

Loading

himdel commented Apr 9, 2020 •

edited

Loading

himdel commented May 1, 2020 •

edited

Loading

himdel commented May 29, 2020 •

edited

Loading

himdel Jun 2, 2020 •

edited

Loading