Repository navigation
feat(ui): add full-row expandable jobs toggle with cookie persistence - #1567
VLukiantsev wants to merge 1 commit into
Conversation
Add a "View in full width" / "Exit full width" toggle button to the queue jobs table on the Resource Observability Dashboard (/resources) that stacks the cluster queues list and jobs table into full-width rows instead of hiding the queue list, and persists the layout preference in the xmc_jobs_full_width browser cookie. TAG=agy CONV=c0655880-9a67-4bdd-aaa7-bc8e8631798f
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a full-width layout preference for the jobs table in the Resource Dashboard, persisting the user's choice via a cookie. It adds a toggle button to the QueueJobsTable component, updates the dashboard's layout styles to support a vertically stacked full-width mode, and includes comprehensive tests. The feedback recommends addressing a flexbox layout issue in resource_dashboard.scss where flex: 1 1 100% on a vertical flex container incorrectly targets height instead of width. Additionally, it is suggested to simplify the toggle button's styling by conditionally rendering mat-flat-button and mat-stroked-button using Angular's control flow rather than manually overriding Material's internal CSS classes.
| .rod-main-content, | ||
| .jobs-side-pane { | ||
| flex: 1 1 100%; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
In a flex container with flex-direction: column-reverse, the main axis is vertical. Therefore, setting flex: 1 1 100% sets the flex-basis (initial height) of .rod-main-content and .jobs-side-pane to 100% of the container's height, rather than their width.
To make both elements stack vertically and span the full width of the container while maintaining their natural height based on content, you should set flex: 1 1 auto (or flex: none) and keep width: 100%.
| .rod-main-content, | |
| .jobs-side-pane { | |
| flex: 1 1 100%; | |
| width: 100%; | |
| } | |
| .rod-main-content, | |
| .jobs-side-pane { | |
| flex: 1 1 auto; | |
| width: 100%; | |
| } |
| @if (showFullWidthToggle()) { | ||
| <button | ||
| type="button" | ||
| mat-stroked-button | ||
| class="full-width-toggle-btn" | ||
| [class.active]="isFullWidth()" | ||
| (click)="toggleFullWidth()" | ||
| > | ||
| <mat-icon aria-hidden="true">{{ fullWidthToggleIcon() }}</mat-icon> | ||
| {{ fullWidthToggleLabel() }} | ||
| </button> | ||
| } |
There was a problem hiding this comment.
Instead of manually overriding Angular Material's stroked button styles (like background color, border color, text color, hover, focus, and ripple states) via custom CSS classes when the button is active, it is much cleaner and more idiomatic to conditionally render either a mat-flat-button (with color="primary") or a mat-stroked-button using Angular's @if / @else control flow.
This ensures that the button natively inherits the correct Material Design styles, hover overlays, focus rings, and ripple colors for both states without requiring complex and fragile CSS overrides.
@if (showFullWidthToggle()) {
@if (isFullWidth()) {
<button
type="button"
mat-flat-button
color="primary"
class="full-width-toggle-btn"
(click)="toggleFullWidth()"
>
<mat-icon aria-hidden="true">{{ fullWidthToggleIcon() }}</mat-icon>
{{ fullWidthToggleLabel() }}
</button>
} @else {
<button
type="button"
mat-stroked-button
class="full-width-toggle-btn"
(click)="toggleFullWidth()"
>
<mat-icon aria-hidden="true">{{ fullWidthToggleIcon() }}</mat-icon>
{{ fullWidthToggleLabel() }}
</button>
}
}
| button.full-width-toggle-btn.mat-mdc-outlined-button { | ||
| border-radius: 9999px; | ||
| border-color: #dadce0; | ||
| background-color: #ffffff; | ||
| color: #1a73e8; | ||
| font-size: 14px; | ||
| font-weight: 500; | ||
| white-space: nowrap; | ||
|
|
||
| .mat-icon { | ||
| color: inherit; | ||
| } | ||
|
|
||
| &.active { | ||
| background-color: #1a73e8; | ||
| border-color: #1a73e8; | ||
| color: #ffffff; | ||
| } | ||
| } |
There was a problem hiding this comment.
By using the native mat-flat-button and mat-stroked-button directives, we can eliminate almost all of the custom CSS overrides. We only need to style the button's border-radius to make it pill-shaped. This also avoids coupling our styles to internal Angular Material classes like .mat-mdc-outlined-button, which are implementation details and subject to change in future library updates.
.full-width-toggle-btn {
border-radius: 9999px;
}There was a problem hiding this comment.
Code Review
This pull request introduces a full-width layout preference for the jobs table in the Resource Dashboard, persisting the user's choice via cookies. The feedback suggests adding defensive null/undefined checks to the document cookie utility functions for non-browser environments, adjusting the flex layout properties to prevent vertical stretching, and avoiding coupling custom styles to internal Angular Material MDC class names.
| export function readJobsFullWidthCookie(doc: Document): boolean { | ||
| const cookieHeader = doc.cookie || ''; | ||
| if (!cookieHeader) { | ||
| return false; | ||
| } | ||
| for (const rawEntry of cookieHeader.split(';')) { | ||
| const [rawKey, ...rawValueParts] = rawEntry.trim().split('='); | ||
| if (rawKey === JOBS_FULL_WIDTH_COOKIE_NAME) { | ||
| return rawValueParts.join('=') === 'true'; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| /** Persists the jobs table full-width layout preference to document cookies. */ | ||
| export function writeJobsFullWidthCookie(doc: Document, isFullWidth: boolean): void { | ||
| doc.cookie = `${JOBS_FULL_WIDTH_COOKIE_NAME}=${isFullWidth}; Path=/; Max-Age=${JOBS_FULL_WIDTH_COOKIE_MAX_AGE_SECONDS}; SameSite=Lax`; | ||
| } |
There was a problem hiding this comment.
To ensure robustness, especially in non-browser environments (such as Server-Side Rendering or unit testing where the DOCUMENT token might be null, undefined, or partially mocked), we should add defensive null/undefined checks to the doc parameter before accessing doc.cookie.
export function readJobsFullWidthCookie(doc: Document | null | undefined): boolean {
const cookieHeader = doc?.cookie || '';
if (!cookieHeader) {
return false;
}
for (const rawEntry of cookieHeader.split(';')) {
const [rawKey, ...rawValueParts] = rawEntry.trim().split('=');
if (rawKey === JOBS_FULL_WIDTH_COOKIE_NAME) {
return rawValueParts.join('=') === 'true';
}
}
return false;
}
/** Persists the jobs table full-width layout preference to document cookies. */
export function writeJobsFullWidthCookie(doc: Document | null | undefined, isFullWidth: boolean): void {
if (!doc) {
return;
}
doc.cookie = `${JOBS_FULL_WIDTH_COOKIE_NAME}=${isFullWidth}; Path=/; Max-Age=${JOBS_FULL_WIDTH_COOKIE_MAX_AGE_SECONDS}; SameSite=Lax`;
}| .rod-main-content, | ||
| .jobs-side-pane { | ||
| flex: 1 1 100%; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
In a column-reverse flex layout, setting flex: 1 1 100% on the child elements forces their main size (height) to start at 100% of the parent container. This can cause unintended vertical stretching, squishing, or overflow issues. Since we want the panes to stack vertically and take their natural content height, we should use flex: 0 0 auto (or flex: none) instead.
| .rod-main-content, | |
| .jobs-side-pane { | |
| flex: 1 1 100%; | |
| width: 100%; | |
| } | |
| .rod-main-content, | |
| .jobs-side-pane { | |
| flex: 0 0 auto; | |
| width: 100%; | |
| } |
| button.full-width-toggle-btn.mat-mdc-outlined-button { | ||
| border-radius: 9999px; | ||
| border-color: #dadce0; | ||
| background-color: #ffffff; | ||
| color: #1a73e8; | ||
| font-size: 14px; | ||
| font-weight: 500; | ||
| white-space: nowrap; | ||
|
|
||
| .mat-icon { | ||
| color: inherit; | ||
| } | ||
|
|
||
| &.active { | ||
| background-color: #1a73e8; | ||
| border-color: #1a73e8; | ||
| color: #ffffff; | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid coupling the custom styles directly to Angular Material's internal MDC class names like .mat-mdc-outlined-button. These classes are implementation details of the library and are subject to change in future versions of Angular Material. Styling .full-width-toggle-btn directly is cleaner and more maintainable.
| button.full-width-toggle-btn.mat-mdc-outlined-button { | |
| border-radius: 9999px; | |
| border-color: #dadce0; | |
| background-color: #ffffff; | |
| color: #1a73e8; | |
| font-size: 14px; | |
| font-weight: 500; | |
| white-space: nowrap; | |
| .mat-icon { | |
| color: inherit; | |
| } | |
| &.active { | |
| background-color: #1a73e8; | |
| border-color: #1a73e8; | |
| color: #ffffff; | |
| } | |
| } | |
| button.full-width-toggle-btn { | |
| border-radius: 9999px; | |
| border-color: #dadce0; | |
| background-color: #ffffff; | |
| color: #1a73e8; | |
| font-size: 14px; | |
| font-weight: 500; | |
| white-space: nowrap; | |
| .mat-icon { | |
| color: inherit; | |
| } | |
| &.active { | |
| background-color: #1a73e8; | |
| border-color: #1a73e8; | |
| color: #ffffff; | |
| } | |
| } |
…ect header actions Intentionally refines two internal design choices from #1567 while preserving identical visual layout and user behavior: - Replace `document.cookie` (`xmc_jobs_full_width`) with `window.localStorage` (`xmc-jobs-full-width`) to align with existing XMC frontend preference conventions (`StatusMessageFilterService`) and avoid sending a client-only layout flag in HTTP `Cookie` headers on API requests. - Replace `showFullWidthToggle` / `isFullWidth` inputs on `QueueJobsTable` with a generic `<ng-content select="[headerActions]" />` slot in `.title-row` so `QueueJobsTable` stays layout-agnostic and `ResourceDashboard` owns the full-width toggle button directly. TAG=agy CONV=23cac48a-f5e0-4dc9-b0fa-c46ba04c7f87
cast.3.webm
Summary
Adds a
"View in full width"/"Exit full width"toggle button to the queue jobs card on the/resources(Resource Observability Dashboard) page that expands the jobs table to the top full-width row with the cluster queues list stacked below (flex-direction: column-reverse), and persists the user's layout preference in thexmc_jobs_full_widthbrowser cookie.Motivation & Context
Wide multi-column job rows in the side-by-side split view can feel cramped, while hiding the cluster queues list altogether prevents inspecting queues alongside jobs. Expanding the jobs card to a full-width top row with the cluster queues list below keeps all dashboard widgets visible and allows persisting the preferred layout in a browser cookie so the view is restored on future visits.
Key Changes
cmd/ui/app/side_panel/contents/queue_jobs_table/):showFullWidthTogglesignal input (defaultfalse) and two-wayisFullWidthsignal model (model<boolean>(false)) onQueueJobsTableso existing side-panel consumers remain unaffected..title-rowswitching between outlined"View in full width"(fullscreenicon) and filled primary"Exit full width"(fullscreen_exiticon).QueueJobsTableHarnessandqueue_jobs_table_test.tsto cover default visibility, icon/label state transitions, active styling, and two-wayisFullWidthmodel updates.cmd/ui/app/resource_dashboard/jobs_layout_preference.ts):readJobsFullWidthCookieandwriteJobsFullWidthCookiefor reading and persistingxmc_jobs_full_width(Path=/; Max-Age=31536000; SameSite=Lax) with 1-to-1 unit tests injobs_layout_preference_test.ts.cmd/ui/app/resource_dashboard/):isJobsFullWidthfromreadJobsFullWidthCookieand synchronized updates viaeffect().[class.full-width]="isJobsFullWidth()"on.rod-split-containerto stack.jobs-side-pane(top,100%width,position: static) and.rod-main-content(bottom,100%width) viaflex-direction: column-reversewithout hiding any widgets.ResourceDashboardHarnessandresource_dashboard_test.tsto verify full-row vertical stacking (jobs pane above queue list), cookie persistence, initial hydration from cookie, and state preservation across Smart Resource Finder queries and tab switches.cmd/ui/screenshot_tests/resource_dashboard/resource_dashboard.screenshot.spec.ts-snapshots/).Assumptions
None — requirements were fully specified.
Verification & Testing
Automated Tests
./.githooks/pre-commit && cd cmd/ui && npm run lint && npm run format:check && npm test -- --watch=false --browsers=ChromeHeadlessNoSandbox && npm run test:screenshotLive Deployment & Manual Testing
tools/dev_env/dev_env.py gke-upgradedeploy/ctk_v2/verify_xmc.py— all 5 sections✅ PASS(14/14 Kubernetes podsRunningandReady, services, Gateway & HTTPRoutes, TLS certificates, and GKE Log Monitor)//cmd/ui:ui_image_push), rolled outdeployment/ui-deployment, and verified/resourcesroute serving.ui-deploymentrolled out cleanly (1/1 Running);/resourcesserves the updated bundle with the"View in full width"toggle button, full-row vertical stacking with the jobs card at the top (flex-direction: column-reverse), andxmc_jobs_full_widthcookie persistence.Live Execution Output / Logs
Related Issues / Bugs
N/A
TAG=agy
CONV=c0655880-9a67-4bdd-aaa7-bc8e8631798f