Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Fix update of vertical menu styles after menu click #1110

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

mzazrivec
Copy link
Contributor

@mzazrivec mzazrivec commented Feb 21, 2020

The layout, styles & classes changed a little bit when the vertical menu was re-done into React.

Previously, the menu items would not be correctly marked as inactive when navigating between sections. See screenshot.
Screenshot from 2020-02-21 11-47-49

The layout, styles & classes changed a little bit when the vertical
menu was re-done into React.
@mzazrivec mzazrivec added the bug label Feb 21, 2020
@mzazrivec mzazrivec requested a review from mturley February 21, 2020 10:19
@miq-bot
Copy link
Member

miq-bot commented Feb 21, 2020

Checked commit mzazrivec@75e7d9f with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.20.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 👍

@himdel
Copy link
Contributor

himdel commented Feb 26, 2020

Matches the menu changes from ManageIQ/manageiq-ui-classic#5854 (and ManageIQ/manageiq-ui-classic#6197 + ManageIQ/manageiq-ui-classic#6463) 👍

Ideally we should be updating the menu to read the router history object instead of manually tweaking this. But this updates the existing code to match the right elements again, so, step forward :).

@himdel
Copy link
Contributor

himdel commented Feb 26, 2020

app/javascript/common/menu.js:21
  const newActiveMenuItem = document.querySelector(`[id="${link_item.menu_item_id}"]`);

won't that need an id prefix update too?

(Because of ui's const getItemId = id => `menu_item_${id}`;)

EDIT: never mind, config.js already has them prefixed since #822

@himdel himdel merged commit d42df0e into ManageIQ:master Feb 26, 2020
@himdel himdel added this to the Sprint 131 Ending Mar 2, 2020 milestone Feb 26, 2020
@himdel himdel self-assigned this Feb 26, 2020
@mturley
Copy link
Contributor

mturley commented Feb 26, 2020

@himdel thanks for merging these, I had been meaning to get around to them

@mzazrivec mzazrivec deleted the fix_vertical_menu_update branch February 27, 2020 10:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants