Skip to content
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.

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

Move tagging-related tabs into subtabs on the Settings/Region screen #2522

Merged
merged 3 commits into from
Oct 27, 2017

Conversation

skateman
Copy link
Member

@skateman skateman commented Oct 26, 2017

To have enough space for the help menu customization I had to free up some space among the tabs on the Settings -> Region screen. The solution proposed by the UX team was to join the tabs related to tagging into a single tab and create a second level of tabs.

I introduced a new :active_subtab key in the sandbox for distinguishing. Unfortunately, the tab switching logic in miq_tabs_init concatenates a tab_id after the URL and it is too widely used. So I introduced a hack: we still send the tab_id for the second level tabs, but we append the parent_tab_id parameter to minimize the number of changes in the controller. I know it is ugly and I'm sorry for this 😢 but it will be gone when we convert this screen to angular.

There were some issues with the flash_msg_div when switching tabs, so as @martinpovolny suggested, I went through all the partials and moved the div to a single place above the first level of tabs.

Before:
screenshot from 2017-10-26 15-31-31

After:
screenshot from 2017-10-26 21-22-01

@martinpovolny and @epwinchell can you please review?

@epwinchell
Copy link
Contributor

@miq-bot add_label formatting/styling, enhancement, fine/no

@epwinchell
Copy link
Contributor

@skateman Tested. Looks good.

@h-kataria
Copy link
Contributor

looks good, can be merged once Travis is green, test failures are not related to changes in this PR

@h-kataria h-kataria self-assigned this Oct 27, 2017
@miq-bot
Copy link
Member

miq-bot commented Oct 27, 2017

Checked commits skateman/manageiq-ui-classic@36d5211~...a743672 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
11 files checked, 6 offenses detected

app/controllers/ops_controller/settings/common.rb

app/views/ops/_all_tabs.html.haml

  • ⚠️ - Line 66 - Block has too many lines. [40/25]

@h-kataria
Copy link
Contributor

merging, test failures are not related to changes in this PR.

@h-kataria h-kataria added this to the Sprint 72 Ending Oct 30, 2017 milestone Oct 27, 2017
@h-kataria h-kataria merged commit ddf6608 into ManageIQ:master Oct 27, 2017
@skateman skateman deleted the ops-2nd-level-tabs branch November 16, 2017 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants