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>
This commit is contained in:
4grayandClaude Opus 5.5 committed 2026-10-13 21:45:29 +02:00
1 parent e3a4ea7259
commit ee35951a5e
4 files changed
+48 -2

No files matched your search

@@ -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);
@@ -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. */
@@ -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
@@ -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'),