Simplify reader filter context - #19
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'reader context panel' to the browse view, featuring interactive chips that display and allow clearing of active filters like queue mode, source, read status, sorting, and search. It also includes significant UI refinements to the toolbar, such as adding descriptive labels to control groups and badges for item counts, alongside corresponding CSS updates for both standard and premium layouts. Feedback focuses on optimizing performance by memoizing the context chips array to prevent unnecessary re-renders and addressing a misleading UI state where the same item count is displayed for different queue modes.
| const readerContextChips: ReaderBrowseContextChip[] = [ | ||
| ...(libraryView === "inbox" | ||
| ? [ | ||
| { | ||
| id: "queue", | ||
| label: "Kolejka", | ||
| value: readerQueueModeLabel, | ||
| tone: "accent" as const, | ||
| ...(readerQueueMode !== "for_you" | ||
| ? { | ||
| onRemove: () => navigateReaderQueueMode("for_you"), | ||
| removeLabel: "Wróć do kolejki Dla mnie", | ||
| } | ||
| : {}), | ||
| }, | ||
| ] | ||
| : [ | ||
| { | ||
| id: "library-view", | ||
| label: "Widok", | ||
| value: getLibraryViewLabel(libraryView), | ||
| tone: "accent" as const, | ||
| }, | ||
| ]), | ||
| { | ||
| id: "source", | ||
| label: activeSourceContextLabel, | ||
| value: activeFeedScopeBaseLabel, | ||
| ...(feedFilter.kind !== "all" | ||
| ? { | ||
| onRemove: () => setReaderFeedFilter({ kind: "all" }), | ||
| removeLabel: `Usuń filtr ${activeSourceContextLabel.toLowerCase()}: ${activeFeedScopeBaseLabel}`, | ||
| } | ||
| : { tone: "muted" as const }), | ||
| }, | ||
| { | ||
| id: "read-scope", | ||
| label: "Zakres", | ||
| value: readerScopeLabel, | ||
| ...(!showReadItems | ||
| ? { | ||
| onRemove: () => setReaderShowReadItems(true), | ||
| removeLabel: "Pokaż także przeczytane artykuły", | ||
| } | ||
| : { tone: "muted" as const }), | ||
| }, | ||
| { | ||
| id: "sort", | ||
| label: "Sortowanie", | ||
| value: readerSortLabel, | ||
| ...(itemSortMode !== "newest" | ||
| ? { | ||
| onRemove: () => setReaderItemSortMode("newest"), | ||
| removeLabel: "Przywróć sortowanie od najnowszych", | ||
| } | ||
| : { tone: "muted" as const }), | ||
| }, | ||
| ...(readerSearchLabel | ||
| ? [ | ||
| { | ||
| id: "search", | ||
| label: "Szukaj", | ||
| value: readerSearchLabel, | ||
| onRemove: () => setReaderItemSearch(""), | ||
| removeLabel: `Wyczyść wyszukiwanie: ${readerSearchLabel}`, | ||
| }, | ||
| ] | ||
| : []), | ||
| ]; |
There was a problem hiding this comment.
The readerContextChips array is recreated on every render of the ChannelLab component, which is a very large component with frequent state updates. Since this array is passed as a prop to ReaderBrowseView, it will cause that component to re-render unnecessarily even if the underlying filter values haven't changed. Consider memoizing this array using useMemo and wrapping the onRemove callbacks in useCallback to improve performance.
| <span className="reader-control-badge">{items.length}</span> | ||
| </button> | ||
| <button className={readerQueueMode === "latest" ? "segment-active" : ""} onClick={() => onReaderQueueModeChange("latest")} type="button"> | ||
| Najnowsze | ||
| </button> | ||
| <button className={readerQueueMode === "all" ? "segment-active" : ""} onClick={() => onReaderQueueModeChange("all")} type="button"> | ||
| <span>Wszystkie</span> | ||
| <span className="reader-control-badge">{items.length}</span> |
There was a problem hiding this comment.
The items.length count is used for both the 'Dla mnie' and 'Wszystkie' queue mode buttons. Since items represents the current filtered queue, both buttons will display the same count regardless of which one is active. This is misleading for users who expect to see the total count of items available in each mode. Consider passing separate counts for each queue mode from the parent component.
1 participant
Summary
aria-label/title, documents removal behavior, and fixes mobile queue controls to collapse to a readable 2x2 grid.Evidence
npm run test:unit:web -- reader-browse-viewpassed.npm run buildpassed.npm run test:unitpassed.RSSMASTER_PLAYWRIGHT_MODULE=C:\Users\user\Desktop\RSSmaster\output\playwright-runtime\node_modules\playwright npm run check:readerpassed.RSSMASTER_PLAYWRIGHT_MODULE=C:\Users\user\Desktop\RSSmaster\output\playwright-runtime\node_modules\playwright npm run check:layoutpassed.Screenshots
output/playwright/page-audit-read-inbox-desktop-1180.pngoutput/playwright/page-audit-read-inbox-tablet-1024.pngoutput/playwright/page-audit-read-inbox-mobile-390.pngUX scorecard
Closes #7