fix(ui): make live panel toggles consistent - #1319
Conversation
Greptile SummaryThis PR introduces independent persisted Groups and Channels intent and makes live-layout disclosures consistent.
Confidence Score: 4/5The cross-route suppression leak should be fixed before merging because a shortcut on one live surface can unexpectedly hide panels on the next surface. Temporary master suppression is held by a root singleton and is consumed by every live layout, but no navigation or component lifecycle clears it; the M3U All view also introduces a non-blocking duplicate panel ID. Files Needing Attention: libs/portal/shared/data-access/src/lib/live-layout-panel-state.service.ts; libs/ui/components/src/lib/channel-list-container/all-channels-view/all-channels-view.component.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
User[Panel action or Cmd/Ctrl+B] --> State[LiveLayoutPanelStateService]
State --> Intent[Persisted Groups and Channels intent]
State --> Suppression[Temporary master suppression]
Intent --> M3U[M3U layout]
Intent --> Portal[Xtream and Stalker layouts]
Intent --> Collections[Favorites and Recent]
Suppression --> M3U
Suppression --> Portal
Suppression --> Collections
M3U --> Panels[Effective Groups and Channels visibility]
Portal --> Panels
Collections --> Panels
Prompt To Fix All With AI### Issue 1
libs/portal/shared/data-access/src/lib/live-layout-panel-state.service.ts:64-72
**Master suppression leaks across routes**
When a user presses Cmd/Ctrl+B and then navigates to another live surface, the root-scoped `masterSuppressed` state remains active, causing the destination's applicable Groups and Channels panels to stay hidden until another shortcut or direct panel action clears it.
### Issue 2
libs/ui/components/src/lib/channel-list-container/all-channels-view/all-channels-view.component.ts:44-46
**Duplicate Channels panel ID**
The M3U All view assigns `live-channels-panel` to both the outer collapsible sidebar and this nested component host, leaving the new ARIA relationship and ID-based DOM or accessibility lookups with two targets.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(ui): ignore hidden panels in master ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| toggleMasterSuppression( | ||
| effectivelyVisiblePanels: readonly LiveLayoutPanel[] | ||
| ): void { | ||
| if (this._masterSuppressed()) { | ||
| this._masterSuppressed.set(false); | ||
| return; | ||
| } | ||
|
|
||
| this._masterSuppressed.set(effectivelyVisiblePanels.length > 0); |
There was a problem hiding this comment.
When a user presses Cmd/Ctrl+B and then navigates to another live surface, the root-scoped masterSuppressed state remains active, causing the destination's applicable Groups and Channels panels to stay hidden until another shortcut or direct panel action clears it.
Context Used: CLAUDE.md (source)
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: libs/portal/shared/data-access/src/lib/live-layout-panel-state.service.ts
Line: 64-72
Comment:
**Master suppression leaks across routes**
When a user presses Cmd/Ctrl+B and then navigates to another live surface, the root-scoped `masterSuppressed` state remains active, causing the destination's applicable Groups and Channels panels to stay hidden until another shortcut or direct panel action clears it.
**Context Used:** CLAUDE.md ([source](https://github.com/4gray/iptvnator/blob/master/CLAUDE.md))
**Knowledge Base Used:**
- [Web Frontend App (apps/web)](https://app.greptile.com/iptvnator/-/custom-context/knowledge-base/4gray/iptvnator/-/docs/web-frontend-app.md)
- [Xtream Codes and Stalker Portal Integration](https://app.greptile.com/iptvnator/-/custom-context/knowledge-base/4gray/iptvnator/-/docs/portal-xtream-stalker.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| host: { | ||
| id: 'live-channels-panel', | ||
| }, |
There was a problem hiding this comment.
The M3U All view assigns live-channels-panel to both the outer collapsible sidebar and this nested component host, leaving the new ARIA relationship and ID-based DOM or accessibility lookups with two targets.
Knowledge Base Used: Web Frontend App (apps/web)
Prompt To Fix With AI
This is a comment left during a code review.
Path: libs/ui/components/src/lib/channel-list-container/all-channels-view/all-channels-view.component.ts
Line: 44-46
Comment:
**Duplicate Channels panel ID**
The M3U All view assigns `live-channels-panel` to both the outer collapsible sidebar and this nested component host, leaving the new ARIA relationship and ID-based DOM or accessibility lookups with two targets.
**Knowledge Base Used:** [Web Frontend App (apps/web)](https://app.greptile.com/iptvnator/-/custom-context/knowledge-base/4gray/iptvnator/-/docs/web-frontend-app.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18c82144a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| readonly isAllChannelsPanelCollapsed = computed( | ||
| () => this.activeView() === 'all' && !this.channelsPanelExpanded() | ||
| ); |
There was a problem hiding this comment.
In the M3U Groups view, Cmd/Ctrl+B makes both effective panel inputs false, but this computed state only collapses the outer sidebar for the all view. The outer element therefore retains its persisted pixel width while both child panels have width zero, leaving a large blank column and continuing to constrain the player; the same occurs on mobile after Groups is responsively suppressed and Channels is hidden. Account for the grouped state by shrinking the outer container to its available restore rails when neither panel is expanded.
Useful? React with 👍 / 👎.
| host: { | ||
| id: 'live-channels-panel', | ||
| }, |
There was a problem hiding this comment.
When the M3U All Channels view renders, this host receives id="live-channels-panel" while its newly changed parent sidebar receives the same ID. The hide and restore controls' aria-controls value consequently resolves to a non-unique target, making the disclosure relationship ambiguous to assistive technology and any DOM code using ID lookup. Assign the ID to only one element representing the collapsible panel.
Useful? React with 👍 / 👎.
1 participant
Summary
LiveLayoutPanelStateServiceand idempotent migration fromlive-sidebar-state.inert, ARIA ownership/state, 40px targets, and temporary Cmd/Ctrl+B suppression.Verification
pnpm nx build webpassed.live-panel-toggles.e2e.tspassed 3/3 for M3U, Xtream, and Stalker, including migration, persistence, mobile suppression, accessibility, zero results, external playback, and radio.i18n:check, release-note validation, skill validation, Prettier, andgit diff --checkpassed.Scope boundaries
Refs #1118