From bc902e97d2e8d13a67d2079094140c8e578c0224 Mon Sep 17 00:00:00 2001 From: Bjoern Michaelsen Date: Fri, 23 Aug 2024 17:20:47 +0200 Subject: [PATCH] feat: narrow, but always open "planned actions" (#1896) - reduced width of "planned actions" aside - always keep the aside open, and give it a primary color background to provide some hint on space separation - reduce whitespace use in planned actions - hide disabled apply button when there is nothing to apply. Note that the "lock message" input is also hidden, if there is no lock: if one adds a lock action to an empty plan, previously the input appeared, while the apply button was already there. The apply button stays disabled, but for different reasons (before: nothing to apply, after: missing lock message) Ref: SRX-MVHTKH --- .../src/assets/_variables.scss | 4 +- .../ui/components/LocksTable/LocksTable.scss | 59 +++---- .../ui/components/LocksTable/LocksTable.tsx | 3 +- .../ReleaseDialog/ReleaseDialog.test.tsx | 14 +- .../components/ServiceLane/ServiceLane.scss | 153 +++++++++--------- .../src/ui/components/SideBar/SideBar.scss | 29 ++-- .../ui/components/SideBar/SideBar.test.tsx | 74 +-------- .../src/ui/components/SideBar/SideBar.tsx | 21 +-- .../ui/components/TopAppBar/TopAppBar.scss | 23 ++- .../src/ui/components/TopAppBar/TopAppBar.tsx | 58 +++---- .../src/ui/components/dropdown/dropdown.scss | 17 +- .../frontend-service/src/ui/utils/store.tsx | 4 - 12 files changed, 192 insertions(+), 267 deletions(-) diff --git a/services/frontend-service/src/assets/_variables.scss b/services/frontend-service/src/assets/_variables.scss index 4e3ae8ad5..387093fad 100644 --- a/services/frontend-service/src/assets/_variables.scss +++ b/services/frontend-service/src/assets/_variables.scss @@ -31,7 +31,7 @@ $small-warn-button-font-size: 8px; $small-warn-button-font-weight: 700; // Sidebar variables -$sidebar-width: 30em; +$sidebar-width: 20em; $sidebar-planned-actions-title-right-centering: 9em; $sidebar-apply-button-height: 3em; $sidebar-show-button-height: 3.5em; @@ -41,7 +41,7 @@ $sidebar-transition-duration-large-screen: 0.4s; // Top App Bar $top-app-bar-padding: 0.5em 0 0.5em $nav-bar-width; // 0.5em on top and bottom each -$top-app-bar-height: 5em; // 5em = 80px +$top-app-bar-height: 10em; $top-app-bar-section-gap: 61px; // Logo // 4em = 64px - Logo_width(40px) = 24px / 2 = 12px diff --git a/services/frontend-service/src/ui/components/LocksTable/LocksTable.scss b/services/frontend-service/src/ui/components/LocksTable/LocksTable.scss index 71e3a8040..c83a83427 100644 --- a/services/frontend-service/src/ui/components/LocksTable/LocksTable.scss +++ b/services/frontend-service/src/ui/components/LocksTable/LocksTable.scss @@ -15,37 +15,40 @@ along with kuberpult. If not, see Copyright freiheit.com*/ @use '@material/data-table/data-table'; -.mdc-data-table__table { - width: 100%; - table-layout: fixed; -} - -.mdc-data-table { +.locks-table { + position: relative; + min-width: fit-content; + max-width: calc(100vw - $sidebar-width - 100px); // full size minus the planned actions bar minus some buffer margin: $locks-list-margin; -} - -.mdc-data-indicator { - background-color: var(--mdc-theme-primary); - color: var(--mdc-theme-on-primary); - height: $locks-list-header-height; - border-radius: $locks-list-header-border-radius; - @extend .text-regular; - - .mdc-data-indicator-header { - display: flex; - padding-inline: $locks-list-header-padding; - } - - .mdc-data-header-title { - @extend .headline1; - } - .mdc-data-indicator-field { - flex: 1; - @extend .sub-headline1; + .mdc-data-table__table { + width: 100%; + table-layout: inherit; } - .mdc-data-indicator-sort-button { - padding-inline: 0; + .mdc-data-indicator { + background-color: var(--mdc-theme-primary); + color: var(--mdc-theme-on-primary); + height: $locks-list-header-height; + border-radius: $locks-list-header-border-radius; + @extend .text-regular; + + .mdc-data-indicator-header { + display: flex; + padding-inline: $locks-list-header-padding; + } + + .mdc-data-header-title { + @extend .headline1; + } + + .mdc-data-indicator-field { + flex: 1; + @extend .sub-headline1; + } + + .mdc-data-indicator-sort-button { + padding-inline: 0; + } } } diff --git a/services/frontend-service/src/ui/components/LocksTable/LocksTable.tsx b/services/frontend-service/src/ui/components/LocksTable/LocksTable.tsx index 7ab79d6bc..5bf0218cc 100644 --- a/services/frontend-service/src/ui/components/LocksTable/LocksTable.tsx +++ b/services/frontend-service/src/ui/components/LocksTable/LocksTable.tsx @@ -19,6 +19,7 @@ import * as React from 'react'; import { Button } from '../button'; import { SortAscending, SortDescending } from '../../../images'; import { useCallback } from 'react'; +import classNames from 'classnames'; export const LocksTable: React.FC<{ headerTitle: string; @@ -38,7 +39,7 @@ export const LocksTable: React.FC<{ sortLocks(locks, sort); }, [locks, sort]); return ( -
+
diff --git a/services/frontend-service/src/ui/components/ReleaseDialog/ReleaseDialog.test.tsx b/services/frontend-service/src/ui/components/ReleaseDialog/ReleaseDialog.test.tsx index fe4c918ff..0aecc5045 100644 --- a/services/frontend-service/src/ui/components/ReleaseDialog/ReleaseDialog.test.tsx +++ b/services/frontend-service/src/ui/components/ReleaseDialog/ReleaseDialog.test.tsx @@ -15,7 +15,7 @@ along with kuberpult. If not, see Copyright freiheit.com*/ import { EnvironmentListItem, ReleaseDialog, ReleaseDialogProps } from './ReleaseDialog'; import { fireEvent, render } from '@testing-library/react'; -import { UpdateAction, UpdateOverview, UpdateRolloutStatus, UpdateSidebar } from '../../utils/store'; +import { UpdateAction, UpdateOverview, UpdateRolloutStatus } from '../../utils/store'; import { Environment, EnvironmentGroup, Priority, Release, RolloutStatus, UndeploySummary } from '../../../api/api'; import { Spy } from 'spy4js'; import { SideBar } from '../SideBar/SideBar'; @@ -452,15 +452,10 @@ describe('Release Dialog', () => { }; describe(`Test automatic cart opening`, () => { - it('Test using direct call to open function', () => { - UpdateSidebar.set({ shown: false }); - UpdateSidebar.set({ shown: true }); - expect(UpdateSidebar.get().shown).toBeTruthy(); - }); + it('Test using direct call to open function', () => {}); describe.each(dataLocks)('click handling', (testcase) => { it('Test using deploy button click simulation ' + testcase.name, () => { - UpdateSidebar.set({ shown: false }); UpdateAction.set({ actions: [] }); setTheStore(testcase); @@ -475,7 +470,6 @@ describe('Release Dialog', () => { ); const result = querySelectorSafe('.env-card-deploy-btn'); fireEvent.click(result); - expect(UpdateSidebar.get().shown).toBeTruthy(); expect(UpdateAction.get().actions).toEqual([ { action: { @@ -505,7 +499,6 @@ describe('Release Dialog', () => { }); it('Test using add lock button click simulation', () => { const testcase = data[0]; - UpdateSidebar.set({ shown: false }); UpdateAction.set({ actions: [] }); setTheStore(testcase); @@ -519,10 +512,9 @@ describe('Release Dialog', () => { release={testcase.rels[0]} /> ); - render(); + render(); const result = querySelectorSafe('.env-card-add-lock-btn'); fireEvent.click(result); - expect(UpdateSidebar.get().shown).toBeTruthy(); expect(UpdateAction.get().actions).toEqual([ { action: { diff --git a/services/frontend-service/src/ui/components/ServiceLane/ServiceLane.scss b/services/frontend-service/src/ui/components/ServiceLane/ServiceLane.scss index 9deb2a6f5..0ca154a26 100644 --- a/services/frontend-service/src/ui/components/ServiceLane/ServiceLane.scss +++ b/services/frontend-service/src/ui/components/ServiceLane/ServiceLane.scss @@ -16,95 +16,102 @@ Copyright freiheit.com*/ @import '../../../assets/variables'; -.service-lane__header { - background: var(--mdc-theme-primary); - color: var(--mdc-theme-on-primary); - display: flex; - align-items: center; - justify-content: space-between; - // the padding here has to be big enough, so that - // the border-radius of the whole lane does not break - padding: 0 4px 0 0; - border-radius: $border-radius-medium; - height: $service-lane-header-height; - - white-space: nowrap; - .service-lane-name { - margin-left: 5px; - } +.service-lane { + position: relative; + min-width: fit-content; + max-width: calc(100vw - $sidebar-width - 100px); // full size minus the planned actions bar minus some buffer - .service-lane-wrapper { - overflow: hidden; - @extend .sub-headline1; + .service-lane__header { + background: var(--mdc-theme-primary); + color: var(--mdc-theme-on-primary); display: flex; align-items: center; - } - - .service-action { - border: 1px solid var(--mdc-theme-on-primary); + justify-content: space-between; + // the padding here has to be big enough, so that + // the border-radius of the whole lane does not break + padding: 0 4px 0 0; border-radius: $border-radius-medium; - margin-left: $service-lane-header-actions-margin-left; - height: $service-lane-header-actions-height; - color: var(--mdc-theme-on-primary); - .mdc-button__label { - padding-left: $service-lane-header-actions-button-padding-left; - @extend .text-bold; - } - } -} + height: $service-lane-header-height; -.service__releases { - display: flex; - margin: $service-lane-releases-margin; -} + white-space: nowrap; + .service-lane-name { + margin-left: 5px; + } -.service-lane__diff { - display: flex; -} + .service-lane-wrapper { + overflow: hidden; + @extend .sub-headline1; + display: flex; + align-items: center; + } -.service-lane__diff--container { - display: flex; - flex-direction: row; - align-self: center; - justify-content: space-evenly; - width: $service-lane-diff-element-width; - opacity: 100%; + .service-action { + border: 1px solid var(--mdc-theme-on-primary); + border-radius: $border-radius-medium; + margin-left: $service-lane-header-actions-margin-left; + height: $service-lane-header-actions-height; + color: var(--mdc-theme-on-primary); + .mdc-button__label { + padding-left: $service-lane-header-actions-button-padding-left; + @extend .text-bold; + } + } + } - .service-lane__diff--number { - @extend .text-bold; + .service__releases { display: flex; - align-items: center; - box-sizing: border-box; - justify-content: space-evenly; + margin: $service-lane-releases-margin; + } - // Copied from figma - width: 24px; - height: 24px; - border: 2px solid $service-lane-diff-element-border-color; - border-radius: $border-radius-large; + .service-lane__diff { + display: flex; } - .service-lane__diff--dot { + .service-lane__diff--container { + display: flex; + flex-direction: row; align-self: center; - box-sizing: border-box; + justify-content: space-evenly; + width: $service-lane-diff-element-width; + opacity: 100%; + + .service-lane__diff--number { + @extend .text-bold; + display: flex; + align-items: center; + box-sizing: border-box; + justify-content: space-evenly; + + // Copied from figma + width: 24px; + height: 24px; + border: 2px solid $service-lane-diff-element-border-color; + border-radius: $border-radius-large; + } - // Copied from figma - width: 3px; - height: 3px; - background: $service-lane-diff-element-border-color; - border-radius: $border-radius-large; + .service-lane__diff--dot { + align-self: center; + box-sizing: border-box; + + // Copied from figma + width: 3px; + height: 3px; + background: $service-lane-diff-element-border-color; + border-radius: $border-radius-large; + } } -} -.service-lane__diff--container:hover { - cursor: pointer; - opacity: 60%; - .service-lane__diff--number { - background-color: var(--mdc-theme-surface, #fff); + + .service-lane__diff--container:hover { + cursor: pointer; + opacity: 60%; + .service-lane__diff--number { + background-color: var(--mdc-theme-surface, #fff); + } } -} -.service-lane__diff--container:active { - .service-lane__diff--number { - transform: translateY(4px); + .service-lane__diff--container:active { + .service-lane__diff--number { + transform: translateY(4px); + } } } diff --git a/services/frontend-service/src/ui/components/SideBar/SideBar.scss b/services/frontend-service/src/ui/components/SideBar/SideBar.scss index 3829c399d..0a7c81184 100644 --- a/services/frontend-service/src/ui/components/SideBar/SideBar.scss +++ b/services/frontend-service/src/ui/components/SideBar/SideBar.scss @@ -37,7 +37,7 @@ Copyright freiheit.com*/ } .mdc-drawer-sidebar-list { - margin: 35px 40px; + margin: 5px; display: flex; flex-direction: column; align-items: flex-start; @@ -48,7 +48,7 @@ Copyright freiheit.com*/ border: 2px solid #f2f5f7; border-radius: 10px; background-color: white; - padding: 20px; // 105px 20px 20px; + padding: 5px; width: 100%; position: relative; display: flex; @@ -119,22 +119,14 @@ Copyright freiheit.com*/ overflow: overlay; position: relative; width: 100%; - background-color: #fafbfc; - height: 75vh; -} - -.actions-cart__lock-message { - background: var(--mdc-theme-background); - position: relative; + background-color: var(--mdc-theme-primary); + height: 85vh; } .mdc-drawer-sidebar .mdc-drawer-sidebar-footer-input { border: 2px solid #f2f5f7; border-radius: 10px; - border-bottom-right-radius: 0px; - border-bottom-left-radius: 0px; - background-color: white; - padding: 20px; + padding: 5px; width: 100%; position: relative; display: flex; @@ -150,6 +142,10 @@ Copyright freiheit.com*/ height: $sidebar-apply-button-height; border-radius: 0 0 0 $border-radius-large; @extend .sub-headline1; + + .sidebar-apply-button-hidden { + display: none; + } } .mdc-drawer__drawer { @@ -168,10 +164,3 @@ Copyright freiheit.com*/ top: 0; width: $sidebar-width; } - -.mdc-drawer-sidebar--hidden { - position: absolute; - right: $sidebar-hidden-displacement; - top: 0; - width: $sidebar-width; -} diff --git a/services/frontend-service/src/ui/components/SideBar/SideBar.test.tsx b/services/frontend-service/src/ui/components/SideBar/SideBar.test.tsx index 6ec2a739e..c25dd95ad 100644 --- a/services/frontend-service/src/ui/components/SideBar/SideBar.test.tsx +++ b/services/frontend-service/src/ui/components/SideBar/SideBar.test.tsx @@ -13,7 +13,7 @@ You should have received a copy of the MIT License along with kuberpult. If not, see . Copyright freiheit.com*/ -import { act, render, renderHook } from '@testing-library/react'; +import { render, renderHook } from '@testing-library/react'; import { TopAppBar } from '../TopAppBar/TopAppBar'; import { MemoryRouter } from 'react-router-dom'; import { BatchAction, LockBehavior, ReleaseTrainRequest_TargetType } from '../../../api/api'; @@ -27,54 +27,6 @@ import { DisplayLock, } from '../../utils/store'; import { ActionDetails, ActionTypes, getActionDetails, SideBar } from './SideBar'; -import { elementQuerySelectorSafe } from '../../../setupTests'; - -describe('Show and Hide Sidebar', () => { - interface dataT { - name: string; - expect: (container: HTMLElement) => HTMLElement | void; - } - - const data: dataT[] = [ - { - name: 'Sidebar is hidden', - expect: (container) => - expect(container.getElementsByClassName('mdc-drawer-sidebar--hidden')[0]).toBeTruthy(), - }, - { - name: 'Sidebar is displayed', - expect: (container) => { - const result = elementQuerySelectorSafe(container, '.mdc-show-button'); - act(() => { - result.click(); - }); - expect(container.getElementsByClassName('mdc-drawer-sidebar--displayed')[0]).toBeTruthy(); - }, - }, - ]; - - const getNode = (overrides?: {}): JSX.Element => { - // given - const defaultProps: any = { - children: null, - }; - return ( - - {' '} - - ); - }; - const getWrapper = (overrides?: {}) => render(getNode(overrides)); - - describe.each(data)(`SideBar functionality`, (testcase) => { - it(testcase.name, () => { - // when - const { container } = getWrapper({}); - // then - testcase.expect(container); - }); - }); -}); describe('Sidebar shows list of actions', () => { interface dataT { @@ -144,10 +96,6 @@ describe('Sidebar shows list of actions', () => { updateActions(testcase.actions); // when const { container } = getWrapper({}); - const result = elementQuerySelectorSafe(container, '.mdc-show-button'); - act(() => { - result.click(); - }); // then expect(container.getElementsByClassName('mdc-drawer-sidebar-list')[0].children).toHaveLength( testcase.expectedNumOfActions @@ -207,10 +155,6 @@ describe('Sidebar test deletebutton', () => { updateActions(testcase.actions); // when const { container } = getWrapper({}); - const result = elementQuerySelectorSafe(container, '.mdc-show-button'); - act(() => { - result.click(); - }); const svg = container.getElementsByClassName('mdc-drawer-sidebar-list-item-delete-icon')[0]; if (svg) { const button = svg.parentElement; @@ -717,9 +661,7 @@ describe('Action details', () => { it(testcase.name, () => { updateActions(testcase.actions); const { container } = getWrapper({}); - expect(container.getElementsByClassName('mdc-drawer-sidebar-header-title')[0].textContent).toBe( - testcase.expectedTitle - ); + expect(container.getElementsByClassName('sub-headline1')[0].textContent).toBe(testcase.expectedTitle); }); }); }); @@ -782,17 +724,13 @@ describe('Action details', () => { it('Create an action initially', () => { updateActions([{ action: { $case: 'undeploy', undeploy: { application: 'test' } } }]); const { container } = getWrapper({}); - expect(container.getElementsByClassName('mdc-drawer-sidebar-header-title')[0].textContent).toBe( - 'Planned Actions (1)' - ); + expect(container.getElementsByClassName('sub-headline1')[0].textContent).toBe('Planned Actions (1)'); }); describe.each(data)('', (testcase) => { it(testcase.name, () => { appendAction(testcase.actions); const { container } = getWrapper({}); - expect(container.getElementsByClassName('mdc-drawer-sidebar-header-title')[0].textContent).toBe( - testcase.expectedTitle - ); + expect(container.getElementsByClassName('sub-headline1')[0].textContent).toBe(testcase.expectedTitle); }); }); describe('Deleting an action from the cart', () => { @@ -807,9 +745,7 @@ describe('Action details', () => { const button = svg.parentElement; if (button) button.click(); } - expect(container.getElementsByClassName('mdc-drawer-sidebar-header-title')[0].textContent).toBe( - expected - ); + expect(container.getElementsByClassName('sub-headline1')[0].textContent).toBe(expected); }); }); }); diff --git a/services/frontend-service/src/ui/components/SideBar/SideBar.tsx b/services/frontend-service/src/ui/components/SideBar/SideBar.tsx index a2433c16f..4cb17aaee 100644 --- a/services/frontend-service/src/ui/components/SideBar/SideBar.tsx +++ b/services/frontend-service/src/ui/components/SideBar/SideBar.tsx @@ -14,7 +14,7 @@ along with kuberpult. If not, see Copyright freiheit.com*/ import { Button } from '../button'; -import { DeleteGray, HideBarWhite } from '../../../images'; +import { DeleteGray } from '../../../images'; import { BatchAction, DeleteEnvironmentTeamLockRequest } from '../../../api/api'; import { deleteAction, @@ -380,8 +380,8 @@ export const SideBarList = (): JSX.Element => { ); }; -export const SideBar: React.FC<{ className?: string; toggleSidebar: () => void }> = (props) => { - const { className, toggleSidebar } = props; +export const SideBar: React.FC<{ className?: string }> = (props) => { + const className = 'mdc-drawer-sidebar--displayed'; //props; const actions = useActions(); const [lockMessage, setLockMessage] = useState(''); const api = useApi; @@ -394,6 +394,7 @@ export const SideBar: React.FC<{ className?: string; toggleSidebar: () => void } } else { title = 'Planned Actions'; } + const lockCreationList = actions.filter( (action) => action.action?.$case === 'createEnvironmentLock' || @@ -476,6 +477,7 @@ export const SideBar: React.FC<{ className?: string; toggleSidebar: () => void } setLockMessage(e.target.value); }, []); + const showApply = useMemo(() => actions.length > 0, [actions.length]); const canApply = useMemo( () => actions.length > 0 && (!newLockExists || lockMessage), [actions.length, lockMessage, newLockExists] @@ -560,16 +562,8 @@ export const SideBar: React.FC<{ className?: string; toggleSidebar: () => void } return (