From 1411e0507a0e986182e556a8b299fe9ffa9e8aa2 Mon Sep 17 00:00:00 2001 From: MariaAga Date: Wed, 3 May 2023 10:37:26 +0200 Subject: [PATCH] Refs #30344 - review fixes --- app/assets/stylesheets/base.scss | 20 +++-- app/assets/stylesheets/navigation.scss | 8 +- .../stylesheets/patternfly_and_overrides.scss | 1 + .../layouts/_application_content.html.erb | 26 +++---- test/integration/compute_profile_js_test.rb | 2 +- test/integration_test_helper.rb | 4 +- .../javascripts/hosts/tableCheckboxes.js | 1 + .../BreadcrumbBar/BreadcrumbBar.scss | 2 + .../react_app/components/Layout/Navigation.js | 76 +++++++++---------- .../react_app/components/Layout/layout.scss | 14 +--- .../routes/common/PageLayout/PageLayout.js | 5 +- 11 files changed, 80 insertions(+), 79 deletions(-) diff --git a/app/assets/stylesheets/base.scss b/app/assets/stylesheets/base.scss index 7152957fd571..b3711c975c36 100644 --- a/app/assets/stylesheets/base.scss +++ b/app/assets/stylesheets/base.scss @@ -13,14 +13,23 @@ html { overflow: auto; height: calc(100% - 70px); margin-left: 250px; + .rails-table-toolbar { + padding-bottom: 0; + display: flex; + flex-wrap: wrap; + justify-content: space-between; + } &.pf-c-page { display: block; grid-template-columns: unset; grid-template-areas: unset; } + } body { + position: absolute; // needed for firefox 100% height bug + overflow: auto; height: 100%; width: 100%; } @@ -170,15 +179,14 @@ select { padding-bottom: 9px; } -.title_filter_parent { - overflow: hidden; // so that the parent will have a height - padding-bottom: 0; - .title_filter { - padding-left: 0; - } +.title_filter { + flex-grow: 1; + padding-left: 0; } #title_action { + flex-grow: 1; + display: flex; padding-left: 8px; padding-bottom: 12px; } diff --git a/app/assets/stylesheets/navigation.scss b/app/assets/stylesheets/navigation.scss index 8f355de5ab8a..769f663da4b0 100644 --- a/app/assets/stylesheets/navigation.scss +++ b/app/assets/stylesheets/navigation.scss @@ -4,11 +4,6 @@ .nav a { font-size: 13px; } - - .user-icon { - margin-right: 10px; - } - .active > a, .open > a { background: linear-gradient( @@ -25,4 +20,7 @@ .pf-c-masthead { background: var(--pf-c-masthead--BackgroundColor) image-url('navbar.png'); + .user-icon { + margin-right: 10px; + } } diff --git a/app/assets/stylesheets/patternfly_and_overrides.scss b/app/assets/stylesheets/patternfly_and_overrides.scss index 75a214c4fcdc..487ecaddbbf1 100644 --- a/app/assets/stylesheets/patternfly_and_overrides.scss +++ b/app/assets/stylesheets/patternfly_and_overrides.scss @@ -132,6 +132,7 @@ a { .btn-toolbar { display: flex; align-items: center; + margin-left: auto; #toolbar-spinner { margin-right: 5px; diff --git a/app/views/layouts/_application_content.html.erb b/app/views/layouts/_application_content.html.erb index 74edd91640cb..753ff794c274 100644 --- a/app/views/layouts/_application_content.html.erb +++ b/app/views/layouts/_application_content.html.erb @@ -1,25 +1,23 @@
- +
+ <% if content_for?(:breadcrumbs) %> + <%= yield(:breadcrumbs) %> + <% else %> + <%= mount_breadcrumbs %> + <% end %> +
<%= yield(:before_search_bar) %> -
-
"> +
+
<%= mount_search_bar if searchable? %> - <%= yield(:search_bar) %>  + <%= yield(:search_bar) %>
-
> +
<% if filter_columns? %>
<%= mount_column_selector %> - <%= yield(:column_selector) %>  + <%= yield(:column_selector) %>
<% end %>
diff --git a/test/integration/compute_profile_js_test.rb b/test/integration/compute_profile_js_test.rb index e3ec994f28d2..41233fa6d60d 100644 --- a/test/integration/compute_profile_js_test.rb +++ b/test/integration/compute_profile_js_test.rb @@ -37,7 +37,7 @@ class ComputeProfileJSTest < IntegrationTestWithJavascript # amazon123 exists in fixture compute_attributes.yml click_link("amazon123 (eu-west-1-EC2)") - assert page.has_selector?('#breadcrumb .active', :text => compute_profiles(:one).name), "#{compute_profiles(:one).name} was expected in the breadcrumb active, but was not found" + assert page.has_selector?('.pf-c-page__main-breadcrumb .active', :text => compute_profiles(:one).name), "#{compute_profiles(:one).name} was expected in the breadcrumb active, but was not found" selected_profile = find("#s2id_compute_attribute_compute_profile_id .select2-chosen").text assert_equal compute_profiles(:one).name, selected_profile diff --git a/test/integration_test_helper.rb b/test/integration_test_helper.rb index 512785516b34..fd3c0ee68882 100644 --- a/test/integration_test_helper.rb +++ b/test/integration_test_helper.rb @@ -55,7 +55,7 @@ def assert_index_page(index_path, title_text, new_link_text = nil, has_search = end def assert_breadcrumb_text(text) - assert page.has_selector?(:xpath, "//div[@id='breadcrumb']//*[contains(.,'#{text}')]"), "#{text} was expected in the div[id='breadcrumb'] tag, but was not found" + assert page.has_selector?(:xpath, "//section//div[contains(@class, 'breadcrumb-bar') or contains(@id, 'breadcrumb')] //*[contains(.,'#{text}')]"), "#{text} was expected in //section//div[contains(@class, 'breadcrumb-bar') or contains(@id, 'breadcrumb')], but was not found" end def assert_new_button(index_path, new_link_text, new_path) @@ -208,7 +208,7 @@ def refute_available_organization(organization) def refute_available_organization_menu(organization) within('.location-menu') do first('button').click - within('.location-menu section ul ul', visible: :all) do + within('.location-menu section ul', visible: :all) do assert page.has_no_link?(organization) end end diff --git a/webpack/assets/javascripts/hosts/tableCheckboxes.js b/webpack/assets/javascripts/hosts/tableCheckboxes.js index b3a5bb370738..5e1a1df6c855 100644 --- a/webpack/assets/javascripts/hosts/tableCheckboxes.js +++ b/webpack/assets/javascripts/hosts/tableCheckboxes.js @@ -279,6 +279,7 @@ function updateCounter() { item.attr('data-original-title', title); item.tooltip({ + container: 'body', trigger: 'hover', }); return false; diff --git a/webpack/assets/javascripts/react_app/components/BreadcrumbBar/BreadcrumbBar.scss b/webpack/assets/javascripts/react_app/components/BreadcrumbBar/BreadcrumbBar.scss index 8c2ce3127c2b..b44b2879a201 100644 --- a/webpack/assets/javascripts/react_app/components/BreadcrumbBar/BreadcrumbBar.scss +++ b/webpack/assets/javascripts/react_app/components/BreadcrumbBar/BreadcrumbBar.scss @@ -2,6 +2,8 @@ .breadcrumb-line { margin-right: -60px; margin-left: -60px; + margin-top: 0; + margin-bottom: 0; } .breadcrumbs-list { diff --git a/webpack/assets/javascripts/react_app/components/Layout/Navigation.js b/webpack/assets/javascripts/react_app/components/Layout/Navigation.js index 776459189f5f..5bc5a75ff969 100644 --- a/webpack/assets/javascripts/react_app/components/Layout/Navigation.js +++ b/webpack/assets/javascripts/react_app/components/Layout/Navigation.js @@ -1,4 +1,4 @@ -import React, { useEffect, useRef, useMemo } from 'react'; +import React, { useState, useEffect, useRef, useMemo } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import { @@ -6,7 +6,6 @@ import { NavList, NavItem, NavExpandable, - NavGroup, NavItemSeparator, } from '@patternfly/react-core'; import { getCurrentPath } from './LayoutHelper'; @@ -24,6 +23,7 @@ const Navigation = ({ setNavigationActiveItem, }) => { const clearTimerRef = useRef(); + const [currentPath, setCurrentPath] = useState(window.location.pathname); useEffect( () => () => { if (clearTimerRef.current) clearTimeout(clearTimerRef.current); @@ -50,10 +50,9 @@ const Navigation = ({ }); }); - const getGroupedItems = useMemo( + const groupedItems = useMemo( () => items.map(({ subItems, ...rest }) => { - const { pathname } = window.location; const groups = []; let currIndex = 0; if (subItems.length) { @@ -69,7 +68,7 @@ const Navigation = ({ } else { groups[currIndex].groupItems.push({ ...sub, - isActive: pathname === sub.href.split('?')[0], + isActive: currentPath === sub.href.split('?')[0], }); } }); @@ -77,10 +76,13 @@ const Navigation = ({ return { ...rest, groups }; }), // eslint-disable-next-line react-hooks/exhaustive-deps - [items.length] + [items.length, currentPath] ); - const groupedItems = getGroupedItems; + const clickAndNavigate = (_onClick, href) => { + _onClick && _onClick(); + setCurrentPath(href); + }; return (