services: Improve layout to PF6

Author: Venefilyn

pkg/systemd/service-details.jsx

@@ -51,6 +51,7 @@ import { ModalError } from 'cockpit-components-inline-notification.jsx';
 
 import './service-details.scss';
 import { KebabDropdown } from "cockpit-components-dropdown";
+import { Icon } from "@patternfly/react-core";
 
 const _ = cockpit.gettext;
 const METRICS_POLL_DELAY = 30000; // 30s
@@ -456,15 +457,19 @@ export class ServiceDetails extends React.Component {
             if (active) {
                 status.push(
                     <div key="running" className="status-running">
-                        <OnRunningIcon className="status-icon" />
+                        <Icon status="success">
+                            <OnRunningIcon className="status-icon" />
+                        </Icon>
                         <span className="status">{ _("Running") }</span>
                         <span className="side-note font-xs">{ _("Active since ") + timeformat.dateTime(this.props.unit.ActiveEnterTimestamp / 1000) }</span>
                     </div>
                 );
             } else {
                 status.push(
                     <div key="stopped" className="status-stopped">
-                        <OffIcon className="status-icon" />
+                        <Icon>
+                            <OffIcon className="status-icon" />
+                        </Icon>
                         <span className="status">{ _("Not running") }</span>
                     </div>
                 );

pkg/systemd/service-details.scss

@@ -26,9 +26,9 @@
   padding-inline-start: 0;
 }
 
-.service-thumbtack-icon {
-  color: var(--pf-v5-global--icon--Color--light);
-}
+// .service-thumbtack-icon {
+//   color: var(--pf-v5-global--icon--Color--light);
+// }
 
 .action-button {
   margin-inline-start: 2rem;
@@ -43,10 +43,11 @@
   color: var(--pf-v5-global--icon--Color--light);
 }
 
+// REVIEW @Venefilyn: Green = good/success; Running or enabled is a status so
+// we should use something that is better suited for informational stuff -- like purple
 .status-running > .status-icon,
 .status-enabled > .status-icon {
-  /* mid-way between PF4 success-100 & 200; then rounded to AA contrast */
-  color: #6da000;
+  color: var(--pf-t--global--border--color--status--success--default);
 }
 
 .status-failed > .status-icon,

pkg/systemd/services.jsx

@@ -25,7 +25,7 @@ import React, { useState, useEffect, useCallback } from "react";
 import { createRoot } from 'react-dom/client';
 import { Flex, FlexItem } from "@patternfly/react-core/dist/esm/layouts/Flex/index.js";
 import { Page, PageSection, } from "@patternfly/react-core/dist/esm/components/Page/index.js";
-import { Card } from "@patternfly/react-core/dist/esm/components/Card/index.js";
+import { Card, CardHeader } from "@patternfly/react-core/dist/esm/components/Card/index.js";
 import { SearchInput } from "@patternfly/react-core/dist/esm/components/SearchInput/index.js";
 import { ToggleGroup, ToggleGroupItem } from "@patternfly/react-core/dist/esm/components/ToggleGroup/index.js";
 import { Toolbar, ToolbarContent, ToolbarFilter, ToolbarItem, ToolbarToggleGroup } from "@patternfly/react-core/dist/esm/components/Toolbar/index.js";
@@ -706,13 +706,15 @@ class ServicesPageBody extends React.Component {
         return (
             <PageSection hasBodyWrapper={false}>
                 <Card isCompact>
-                    <ServicesPageFilters activeStateDropdownOptions={activeStateDropdownOptions}
-                                         fileStateDropdownOptions={fileStateDropdownOptions}
-                                         filtersRef={this.filtersRef}
-                                         loadingUnits={this.props.isLoading}
-                                         options={cockpit.location.options}
-                                         onOptionsChanged={this.onOptionsChanged}
-                    />
+                    <CardHeader id='services-card-header'>
+                        <ServicesPageFilters activeStateDropdownOptions={activeStateDropdownOptions}
+                                            fileStateDropdownOptions={fileStateDropdownOptions}
+                                            filtersRef={this.filtersRef}
+                                            loadingUnits={this.props.isLoading}
+                                            options={cockpit.location.options}
+                                            onOptionsChanged={this.onOptionsChanged}
+                        />
+                    </CardHeader>
                     <ServicesList key={cockpit.format("$0-list", activeTab)}
                                   isTimer={activeTab == 'timer'}
                                   filtersRef={this.filtersRef}
@@ -865,7 +867,8 @@ const ServicesPageFilters = ({
         </ToolbarToggleGroup>;
 
     return (
-        <Toolbar data-loading={loadingUnits}
+        <Toolbar
+                data-loading={loadingUnits}
                  clearAllFilters={onClearAllFilters}
                  className="pf-m-sticky-top ct-compact services-toolbar"
                  id="services-toolbar"
@@ -904,7 +907,7 @@ const ServicesPage = () => {
 
     return (
         <WithDialogs>
-            <Page>
+            <Page className='no-masthead-sidebar'>
                 {cockpit.location.path.length == 0 &&
                 <PageSection hasBodyWrapper={false} className="services-header">
                     <Flex>

pkg/systemd/services.scss

@@ -4,8 +4,8 @@
 // The service list lacks a top border.
 // However, the toolbar also lacks a border and is sticky, so add a bottom
 // border to the toolbar to separate the toolbar and list.
-.services-toolbar {
-  border-block-end: 1px solid var(--pf-v5-global--BorderColor--100);
+.pf-m-grid-lg.pf-v6-c-table tbody:where(.pf-v6-c-table__tbody):first-of-type {
+  border-block-start: var(--pf-v6-c-table__tbody--responsive--border-width--base) solid var(--pf-v6-c-table--responsive--BorderColor);
 }
 
 // Magic pixie dust to make the list a bit faster and more furious
@@ -32,7 +32,7 @@
 
 .service-unit-file-state {
   display: inline-block;
-  font-size: var(--pf-v5-global--FontSize--sm);
+  // font-size: var(--pf-t--global--font--size--sm);
   min-inline-size: 5rem;
   text-align: center;
 }
@@ -46,19 +46,6 @@
 .pf-v6-c-nav__link > .ct-exclamation-circle {
   margin-inline-start: 0.5rem;
 }
-
-.services-header.pf-v6-c-page__main-nav {
-  padding-block-end: 1rem;
-  padding-inline-end: 0;
-
-  > .pf-v6-l-flex {
-    // Add spacing between the tabs and the 'Create timer' button in mobile
-    row-gap: var(--pf-v5-global--spacer--sm);
-    // Align 'Create timer' button with the right side of the services list card
-    margin-inline-end: var(--pf-v5-c-page__main-section--PaddingRight);
-  }
-}
-
 // Add some spacing between the icon and the status string in the list
 .service-unit-status-failed > .ct-exclamation-circle {
   margin-inline-end: 0.5rem;
@@ -68,23 +55,26 @@
   min-inline-size: 20rem;
   display: grid;
   grid-template-columns: repeat(auto-fill, minmax(5rem, 25rem));
-  grid-gap: var(--pf-v5-global--spacer--sm);
+  grid-gap: var(--pf-t--global--spacer--sm);
 
   .service-unit-description {
     font-weight: bold;
   }
 }
 
-.service-thumbtack-icon-color {
-  color: var(--pf-v5-global--icon--Color--light);
+// FIXME @Venefilyn: Long-named services causes the cell to exceed its length
+// as it won't wrap correctly. It does seem like (inline-)flex itself cause the
+// issues. For now we go to how it was in PF5 and set it to display: block.
+#services .service-unit-id {
+  --pf-v6-c-button--Display: block;
 }
 
 .service-unit-triggers {
   min-inline-size: 20ch;
 }
 
 .pf-v6-c-table .service-unit-second-column {
-  --pf-v5-c-table--cell--Width: 20%;
+  --pf-v6-c-table--cell--Width: 20%;
 }
 
 // Don't show labels from mobile mode

test/verify/check-system-services

@@ -61,7 +61,7 @@ class TestServices(testlib.MachineCase):
 
     def do_action(self, action):
         self.browser.click(".service-top-panel button.pf-v6-c-menu-toggle")
-        self.browser.click(f".service-top-panel .pf-v6-c-menu__list-item:contains('{action}') button")
+        self.browser.click(f"#services-page .pf-v6-c-menu__list-item:contains('{action}') button")
 
     def check_service_details(self, statuses, actions, enabled, onoff=True, kebab=True):
         with self.browser.wait_timeout(60):
@@ -73,11 +73,11 @@ class TestServices(testlib.MachineCase):
 
         if kebab:
             self.browser.click(".service-top-panel button.pf-v6-c-menu-toggle")
-            self.browser.wait_text(".service-top-panel .pf-v6-c-menu__list", "".join(actions))
+            self.browser.wait_text("#services-page .pf-v6-c-menu__list", "".join(actions))
             # Click away to close the pf-v6-c-menu__list
             self.browser.click(".service-top-panel button.pf-v6-c-menu-toggle")
         else:
-            self.browser.wait_not_present(".service-top-panel .pf-v6-c-menu__list")
+            self.browser.wait_not_present("#services-page .pf-v6-c-menu__list")
 
     def check_service_on(self, expect_reload=True):
         self.check_service_details(
@@ -630,17 +630,17 @@ WantedBy=default.target
             # wait for layout to settle
             time.sleep(1)
             b.click(".service-top-panel button.pf-v6-c-menu-toggle")
-            b.wait_in_text(".service-top-panel .pf-v6-c-menu__list", "Disallow running")
+            b.wait_in_text("#services-page .pf-v6-c-menu__list", "Disallow running")
             b.assert_pixels_in_current_layout("#service-details-unit", "services-kebabdropdown-rtl")
             # Close menu
             b.click(".service-top-panel button.pf-v6-c-menu-toggle")
             b.wait_not_present(".pf-v6-c-menu__list")
             b.set_layout("desktop")
             b.click(".service-top-panel button.pf-v6-c-menu-toggle")
-            b.wait_in_text(".service-top-panel .pf-v6-c-menu__list", "Disallow running")
+            b.wait_in_text("#services-page .pf-v6-c-menu__list", "Disallow running")
             # check if our custom kebab menu renders properly
             b.assert_pixels("#service-details-unit", "services-kebabdropdown", skip_layouts=['rtl'])
-            b.wait_not_in_text(".service-top-panel .pf-v6-c-menu__list", "Delete")
+            b.wait_not_in_text("#services-page .pf-v6-c-menu__list", "Delete")
             b.click(".pf-v6-c-breadcrumb a:contains('Services')")
 
     @testlib.nondestructive