Skip to content

feat: behavioral-audit UI and interaction fixes (front-end, 2/2) - #214

Closed
chhoumann wants to merge 6 commits into
chhoumann/audit-logic-fixesfrom
chhoumann/audit-ui-fixes
Closed

chhoumann wants to merge 6 commits into
chhoumann/audit-logic-fixesfrom
chhoumann/audit-ui-fixes

Conversation

@chhoumann

Copy link
Copy Markdown
Owner

Summary

Part 2 of 2 from the behavioral audit: the UI / interaction fixes (player, chapters, context menu, playlists, podcast view). Stacked on #213 (base is chhoumann/audit-logic-fixes); it will retarget to master once #213 merges.

Each change was driven through a real Obsidian dev vault, not just unit tests, because layout/render/interaction bugs don't show up in jsdom. That pass caught one regression in this set (a playlist-tile name overlapping its count), which is fixed here.

No audit tooling or spreadsheets are included; only code and tests.

What's added / fixed

Player, chapters, accessible seek

  • Video playback gets a fullscreen button (no native controls, with an iOS fallback).
  • Chapter-list active highlight now tracks playback time (the reactive statement had no dependencies) and follows a clicked chapter; stale chapters are cleared on episode switch and out-of-order fetches are ignored.
  • Progress bar has an accessible name and a human-readable aria-valuetext; chapter list gets aria-controls and descriptive seek labels.
  • Restore-position and end-of-episode cleanup match by composite key, so a same-titled episode from another podcast isn't resumed or removed by mistake.

Context menu, playlists, validation

  • A keyboard/mobile reachable overflow (kebab) button opens the episode context menu via showAtPosition (it was right-click only).
  • The playlist context-menu separator only renders when there are playlists; the dropped download promise is settled.
  • Playlist/queue grid tiles show their name (and the card grows so the name can't clip or overlap the count).
  • Playlist name validation: empty names are rejected and an existing playlist is never overwritten (data loss); the list is keyed so per-row delete state can't drift; the icon dropdown resets after add.
  • Removed unreachable repeat-toggle dead code; enlarged the reorder-modal action buttons to a comfortable touch target.

Podcast view

  • Refresh actually refreshes all feeds in the Latest Episodes view; duplicate loading indicators for a selected feed are avoided.
  • A whitespace-only search is treated as empty; an open playlist live-updates; feed-load and search failures (invalid feed URL, iTunes errors) are surfaced instead of failing silently.

Verification

  • lint, format:check, typecheck, build: clean.
  • check:a11y (svelte-check): 0 errors / 0 warnings.
  • Vitest: 732 passing (includes new ChapterList and context-menu tests).
  • Real Obsidian (dev vault): progress-bar aria, reorder-modal touch targets (36px), playlist/queue tile names with no overlap (the regression this caught, now fixed), playlist empty-name and duplicate-name (data-loss) guards, and the focusable kebab affordance were all confirmed live with no console errors. Chapter reactivity, video fullscreen, and the menu rendering from a real click are covered by the Testing-Library unit tests (Obsidian's Menu doesn't render from synthetic events, so it isn't screenshot-confirmable in the harness; the established right-click path is equally unobservable there).

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 21, 2026 •

Copy link
Copy Markdown

Deploying podnotes with  Cloudflare Pages  Cloudflare Pages

Latest commit: fbc8c7a
Status: ✅  Deploy successful!
Preview URL: https://a01cf32b.podnotes.pages.dev
Branch Preview URL: https://chhoumann-audit-ui-fixes.podnotes.pages.dev

View logs

@chhoumann
chhoumann force-pushed the chhoumann/audit-ui-fixes branch from 09d1bcf to 44e73a2 Compare June 21, 2026 21:38

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 09d1bcfe35

ℹ️ 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".

Comment thread src/ui/PodcastView/spawnEpisodeContextMenu.ts Outdated
@chhoumann

Copy link
Copy Markdown
Owner Author

@codex review

@chhoumann
chhoumann force-pushed the chhoumann/audit-ui-fixes branch from 44e73a2 to 9ebdb84 Compare June 21, 2026 21:54

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44e73a2d81

ℹ️ 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".

Comment thread src/ui/PodcastView/spawnEpisodeContextMenu.ts Outdated
chhoumann added a commit that referenced this pull request Jun 21, 2026
The queue identifies episodes by title everywhere (queue.add dedupes and
queue.remove filters by title), so the context-menu membership check, the
reorder index lookup, and the end-of-episode queue cleanup must match by title
too. A composite-key check offered "Add to Queue" for a same-titled episode
from another podcast, which queue.add then no-opped. Addresses the Codex
review on #214.
@chhoumann
chhoumann force-pushed the chhoumann/audit-ui-fixes branch from 9ebdb84 to 63edbde Compare June 21, 2026 22:09
@chhoumann

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 63edbde1a4

ℹ️ 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".

Comment thread src/ui/PodcastView/spawnEpisodeContextMenu.ts Outdated
Comment thread src/ui/PodcastView/EpisodePlayer.svelte Outdated
chhoumann added a commit that referenced this pull request Jun 21, 2026
The queue identifies episodes by title everywhere (queue.add dedupes and
queue.remove filters by title), so the context-menu membership check, the
reorder index lookup, and the end-of-episode queue cleanup must match by title
too. A composite-key check offered "Add to Queue" for a same-titled episode
from another podcast, which queue.add then no-opped. Addresses the Codex
review on #214.
chhoumann added a commit that referenced this pull request Jun 21, 2026
Add isSameStoredEpisode, which matches a stored playlist/favorite entry by
composite key and, for LEGACY entries saved without podcastName, by title alone
(without matching a different podcast's same title). Use it for the favorites and
playlist context-menu checks and the end-of-episode playlist cleanup, so a legacy
entry is no longer treated as absent (duplicated on add, skipped on cleanup).
Addresses the Codex review on #214.
@chhoumann
chhoumann force-pushed the chhoumann/audit-ui-fixes branch from 63edbde to 9947269 Compare June 21, 2026 22:29
@chhoumann

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9947269e45

ℹ️ 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".

Comment thread src/ui/PodcastView/PodcastView.svelte
- add a fullscreen button to video playback (no native controls)
- fix the chapter-list active-row highlight to track currentTime (the
  reactive statement had no dependencies) and follow a clicked chapter
- clear stale chapters on episode switch and ignore out-of-order fetches
- give the progress bar an accessible name + human-readable aria-valuetext,
  and the chapter list aria-controls + descriptive seek labels
- restore by composite key and end-of-episode cleanup by composite key, so a
  same-titled episode from another podcast isn't resumed/removed by mistake
…tion

- add a keyboard/mobile reachable overflow (kebab) affordance that opens the
  episode context menu via showAtPosition (was right-click only)
- only render the playlist context-menu separator when there are playlists
- settle the dropped download promise from the context menu
- show the playlist/queue name on grid tiles (and stop the name from
  clipping/overlapping the count by letting the card grow)
- validate playlist names: reject empty names and refuse to overwrite an
  existing playlist (data loss); key the list so per-row delete state can't
  drift; reset the icon dropdown after add
- remove the unreachable repeat-toggle dead code
- enlarge the reorder-modal action buttons to a comfortable touch target
- make Refresh actually refresh all feeds in the Latest Episodes view
- avoid duplicate loading indicators for a selected feed
- treat a whitespace-only search as empty and live-update an open playlist
- surface feed-load and search failures (invalid feed URL, iTunes errors) to
  the user instead of failing silently
The queue identifies episodes by title everywhere (queue.add dedupes and
queue.remove filters by title), so the context-menu membership check, the
reorder index lookup, and the end-of-episode queue cleanup must match by title
too. A composite-key check offered "Add to Queue" for a same-titled episode
from another podcast, which queue.add then no-opped. Addresses the Codex
review on #214.
Add isSameStoredEpisode, which matches a stored playlist/favorite entry by
composite key and, for LEGACY entries saved without podcastName, by title alone
(without matching a different podcast's same title). Use it for the favorites and
playlist context-menu checks and the end-of-episode playlist cleanup, so a legacy
entry is no longer treated as absent (duplicated on add, skipped on cleanup).
Addresses the Codex review on #214.
A custom playlist named like a built-in (Queue/Favorites/Local Files/Played)
is ambiguous because those tiles come from separate stores, so an open custom
playlist could silently switch to the built-in list on refresh. Reject those
names on create. Addresses the follow-up Codex review on #214.
@chhoumann
chhoumann force-pushed the chhoumann/audit-ui-fixes branch from 9947269 to fbc8c7a Compare June 21, 2026 22:46
@chhoumann

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: fbc8c7af6d

ℹ️ 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".

@chhoumann
chhoumann deleted the branch chhoumann/audit-logic-fixes June 22, 2026 05:39
@chhoumann chhoumann closed this Jun 22, 2026
chhoumann added a commit that referenced this pull request Jun 22, 2026
The queue identifies episodes by title everywhere (queue.add dedupes and
queue.remove filters by title), so the context-menu membership check, the
reorder index lookup, and the end-of-episode queue cleanup must match by title
too. A composite-key check offered "Add to Queue" for a same-titled episode
from another podcast, which queue.add then no-opped. Addresses the Codex
review on #214.
chhoumann added a commit that referenced this pull request Jun 22, 2026
Add isSameStoredEpisode, which matches a stored playlist/favorite entry by
composite key and, for LEGACY entries saved without podcastName, by title alone
(without matching a different podcast's same title). Use it for the favorites and
playlist context-menu checks and the end-of-episode playlist cleanup, so a legacy
entry is no longer treated as absent (duplicated on add, skipped on cleanup).
Addresses the Codex review on #214.
chhoumann added a commit that referenced this pull request Jun 22, 2026
A custom playlist named like a built-in (Queue/Favorites/Local Files/Played)
is ambiguous because those tiles come from separate stores, so an open custom
playlist could silently switch to the built-in list on refresh. Reject those
names on create. Addresses the follow-up Codex review on #214.
chhoumann added a commit that referenced this pull request Jun 22, 2026
* feat(player): video fullscreen, live chapter highlight, accessible seek

- add a fullscreen button to video playback (no native controls)
- fix the chapter-list active-row highlight to track currentTime (the
  reactive statement had no dependencies) and follow a clicked chapter
- clear stale chapters on episode switch and ignore out-of-order fetches
- give the progress bar an accessible name + human-readable aria-valuetext,
  and the chapter list aria-controls + descriptive seek labels
- restore by composite key and end-of-episode cleanup by composite key, so a
  same-titled episode from another podcast isn't resumed/removed by mistake

* feat(ui): reachable context menu, visible playlist names, name validation

- add a keyboard/mobile reachable overflow (kebab) affordance that opens the
  episode context menu via showAtPosition (was right-click only)
- only render the playlist context-menu separator when there are playlists
- settle the dropped download promise from the context menu
- show the playlist/queue name on grid tiles (and stop the name from
  clipping/overlapping the count by letting the card grow)
- validate playlist names: reject empty names and refuse to overwrite an
  existing playlist (data loss); key the list so per-row delete state can't
  drift; reset the icon dropdown after add
- remove the unreachable repeat-toggle dead code
- enlarge the reorder-modal action buttons to a comfortable touch target

* fix(podcastview): search, refresh, and feed-load feedback

- make Refresh actually refresh all feeds in the Latest Episodes view
- avoid duplicate loading indicators for a selected feed
- treat a whitespace-only search as empty and live-update an open playlist
- surface feed-load and search failures (invalid feed URL, iTunes errors) to
  the user instead of failing silently

* fix(queue): keep queue context-menu and cleanup checks title-based

The queue identifies episodes by title everywhere (queue.add dedupes and
queue.remove filters by title), so the context-menu membership check, the
reorder index lookup, and the end-of-episode queue cleanup must match by title
too. A composite-key check offered "Add to Queue" for a same-titled episode
from another podcast, which queue.add then no-opped. Addresses the Codex
review on #214.

* fix(playlists): match legacy (title-only) entries in favorites/playlists

Add isSameStoredEpisode, which matches a stored playlist/favorite entry by
composite key and, for LEGACY entries saved without podcastName, by title alone
(without matching a different podcast's same title). Use it for the favorites and
playlist context-menu checks and the end-of-episode playlist cleanup, so a legacy
entry is no longer treated as absent (duplicated on add, skipped on cleanup).
Addresses the Codex review on #214.

* fix(playlists): reject reserved built-in names when creating a playlist

A custom playlist named like a built-in (Queue/Favorites/Local Files/Played)
is ambiguous because those tiles come from separate stores, so an open custom
playlist could silently switch to the built-in list on refresh. Reject those
names on create. Addresses the follow-up Codex review on #214.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant