From ee35951a5e11e68c9465f329599a30f029de319d Mon Sep 17 00:00:00 2001
From: 4gray <serega05@gmail.com>
Date: Tue, 13 Oct 2026 21:45:29 +0200
Subject: [PATCH] fix(sources): name the Sources sort group; refit the switcher
 on resize

Local review on the PR round:

- Greptile: the Sources sort menu's radio group had no accessible name,
  unlike every other menu group (UI guidelines). It is named by its
  section, "Sort by".
- Codex: the switcher popover capped its height once, when it opened, so
  shrinking the window while it was open left the footer and the lower
  rows out of reach. A window resize refits the open popover.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 .../playlist-switcher.component.spec.ts       | 24 +++++++++++++++++++
 .../playlist-switcher.component.ts            | 15 +++++++++++-
 .../workspace-sources.component.html          |  7 +++++-
 .../workspace-sources.component.spec.ts       |  4 ++++
 4 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher.component.spec.ts b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher.component.spec.ts
index ee85ebda3..f2217483c 100644
--- a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher.component.spec.ts
+++ b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher.component.spec.ts
@@ -116,6 +116,30 @@ describe('PlaylistSwitcherComponent', () => {
         expect(document.activeElement).toBe(trigger());
     });
 
+    it('refits the open popover when the window shrinks', () => {
+        const height = Object.getOwnPropertyDescriptor(window, 'innerHeight');
+        const setHeight = (value: number) =>
+            Object.defineProperty(window, 'innerHeight', {
+                configurable: true,
+                value,
+            });
+        try {
+            setHeight(900);
+            trigger().click();
+            fixture.detectChanges();
+            const opened = fixture.componentInstance.maxHeight();
+
+            setHeight(600);
+            window.dispatchEvent(new Event('resize'));
+            expect(fixture.componentInstance.maxHeight()).not.toBe(opened);
+            expect(
+                parseInt(fixture.componentInstance.maxHeight() ?? '', 10)
+            ).toBeLessThanOrEqual(600);
+        } finally {
+            if (height) Object.defineProperty(window, 'innerHeight', height);
+        }
+    });
+
     it('opens on a ⌘P / Ctrl+P request while it is on screen', () => {
         const switcherOpen = TestBed.inject(PlaylistSwitcherOpenService);
         expect(switcherOpen.available).toBe(true);
diff --git a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher.component.ts b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher.component.ts
index d6c1c3080..ddec7bd97 100644
--- a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher.component.ts
+++ b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher.component.ts
@@ -68,6 +68,7 @@ const VIEWPORT_MARGIN = 12;
     templateUrl: './playlist-switcher.component.html',
     styleUrls: ['./playlist-switcher.component.scss'],
     changeDetection: ChangeDetectionStrategy.OnPush,
+    host: { '(window:resize)': 'onWindowResize()' },
 })
 export class PlaylistSwitcherComponent {
     private readonly playlistContext = inject(PlaylistContextFacade);
@@ -148,12 +149,24 @@ export class PlaylistSwitcherComponent {
     }
 
     open(): void {
+        this.fitToViewport();
+        this.isOpen.set(true);
+    }
+
+    /** A window resized while the popover is open refits it. */
+    onWindowResize(): void {
+        if (this.isOpen()) {
+            this.fitToViewport();
+        }
+    }
+
+    /** Caps the popover to the room below the trigger; only its list scrolls. */
+    private fitToViewport(): void {
         const bottom =
             this.trigger().nativeElement.getBoundingClientRect().bottom;
         this.maxHeight.set(
             `${Math.max(240, window.innerHeight - bottom - 8 - VIEWPORT_MARGIN)}px`
         );
-        this.isOpen.set(true);
     }
 
     /** Closes the popover; focus goes back to the trigger. */
diff --git a/libs/workspace/shell/feature/src/lib/workspace-sources/workspace-sources.component.html b/libs/workspace/shell/feature/src/lib/workspace-sources/workspace-sources.component.html
index 07dc81a96..235a9d1cd 100644
--- a/libs/workspace/shell/feature/src/lib/workspace-sources/workspace-sources.component.html
+++ b/libs/workspace/shell/feature/src/lib/workspace-sources/workspace-sources.component.html
@@ -49,7 +49,12 @@
                     class="app-menu"
                     xPosition="before"
                 >
-                    <div role="group">
+                    <div
+                        role="group"
+                        [attr.aria-label]="
+                            'WORKSPACE.REFINE_SORT_SECTION' | translate
+                        "
+                    >
                         @for (option of sortOptions; track $index) {
                             <button
                                 mat-menu-item
diff --git a/libs/workspace/shell/feature/src/lib/workspace-sources/workspace-sources.component.spec.ts b/libs/workspace/shell/feature/src/lib/workspace-sources/workspace-sources.component.spec.ts
index 1636327c5..a22d43aae 100644
--- a/libs/workspace/shell/feature/src/lib/workspace-sources/workspace-sources.component.spec.ts
+++ b/libs/workspace/shell/feature/src/lib/workspace-sources/workspace-sources.component.spec.ts
@@ -293,6 +293,10 @@ describe('WorkspaceSourcesComponent', () => {
             ...Array(7).fill('menuitemradio'),
             'menuitemcheckbox',
         ]);
+        // The radio group is named by its section, as every menu group is.
+        expect(
+            rows[0].closest('[role="group"]')?.getAttribute('aria-label')
+        ).toBe('WORKSPACE.REFINE_SORT_SECTION');
         expect(rows.map((row) => row.getAttribute('aria-checked'))).toEqual([
             'true',
             ...Array(7).fill('false'),