feat(theme): add AMOLED-Strix theme with semantic CSS token system#2697
Closed
RAFSuNX wants to merge 8 commits intoseerr-team:developfrom
Closed
feat(theme): add AMOLED-Strix theme with semantic CSS token system#2697RAFSuNX wants to merge 8 commits intoseerr-team:developfrom
RAFSuNX wants to merge 8 commits intoseerr-team:developfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a theme system (including an AMOLED "amoled-strix" theme), ThemeProvider/useTheme, appearance settings/pages, a new AmoledNavbar and HeroSlider, a "glass" button variant, many theme-aware styling/token swaps, gradient overlay refactors, and multiple AMOLED-specific rendering paths across detail pages. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as App UI
participant ThemeProv as ThemeProvider
participant DOM as Document (data-theme)
participant Comp as Theme-aware Components
User->>App: open Appearance settings
App->>ThemeProv: request current theme
ThemeProv-->>App: theme + setTheme()
User->>App: selects theme
App->>ThemeProv: call setTheme(themeId)
ThemeProv->>DOM: set data-theme attribute
ThemeProv-->>App: context updated
App->>Comp: re-render (useTheme)
Comp-->>App: render AMOLED variants/backdrops
sequenceDiagram
participant App as App UI
participant Hero as HeroSlider
participant API as /api/v1/discover/trending
participant Cache as CachedImage
participant Modal as RequestModal
App->>Hero: mount
Hero->>API: fetch trending (SWR)
API-->>Hero: trending items
Hero->>Cache: preload backdrops
Hero-->>App: display rotating slides
User->>Hero: click Request
Hero->>Modal: open RequestModal
Modal-->>Hero: onComplete (update status)
Hero->>App: update action state
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
RAFSuNX
force-pushed
the
feat/theming-system
branch
from
e1b55e6 to
56a92b0
Compare
Author
|
Hey! I'm not a regular contributor and this is actually my first PR to the project, so apologies if anything isn't quite right. I just really wanted an AMOLED theme option for use on OLED screens - the default UI looks great, I have no complaints there, I just wanted a pure black variant to take advantage of the display. Thought it might be useful for others too, so figured I'd share it rather than keep it to myself. Hopefully the devs get a chance to look it over. Happy to make any changes if anything needs adjusting! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/Login/JellyfinLogin.tsx (1)
124-130:⚠️ Potential issue | 🔴 CriticalRestore the
!flag on these background utilities.The
input[type='text']andinput[type='password']selectors in globals.css (line 377–381) applybg-gray-700with@apply. This attribute selector has higher specificity than the plainbg-inputclass, sobg-gray-700will override it at runtime. The codebase elsewhere (e.g.,RequestList.tsx) already uses!bg-inputfor this exact reason.Required fix
- className="bg-input placeholder:text-gray-400" + className="!bg-input placeholder:text-gray-400"Apply the same change to the password field at line ~152.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Login/JellyfinLogin.tsx` around lines 124 - 130, The background utility on the input Fields is being overridden by the input[type='text'] / input[type='password'] selector, so restore the important flag by changing the className on the Field with id="username" (and the corresponding Field with id="password") in the JellyfinLogin component to use "!bg-input" instead of "bg-input" (keep the other classes like "placeholder:text-gray-400"); this mirrors the existing fix used in RequestList.tsx and ensures the intended background class wins specificity.src/pages/_app.tsx (1)
203-223:⚠️ Potential issue | 🟡 MinorSet the initial theme during SSR or via an inline script to prevent a flash for users with persisted preferences.
The
ThemeProviderappliesdata-themeinside auseEffect, which runs after hydration. Since_document.tsxdoes not set an initialdata-themeattribute and no inline script runs before paint, users with the 'amoled-strix' preference will see the default theme briefly when refreshing before the effect sets the correct theme. Add an inline script in_document.tsxthat reads localStorage and setsdocument.documentElement.setAttribute('data-theme', ...)before the page renders, or pass the theme from the server during SSR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/_app.tsx` around lines 203 - 223, The app flashes the wrong theme because ThemeProvider only sets the data-theme inside a client-side useEffect; to fix, add an inline script in the custom document that runs before paint and sets document.documentElement.setAttribute('data-theme', <value>) by reading the same localStorage key your ThemeProvider uses (or alternatively pass the persisted theme from SSR into currentSettings and render it on the server); target the ThemeProvider/data-theme behavior and ensure the inline script reads the identical storage key and sets the attribute synchronously so the correct theme is present on first paint.
🧹 Nitpick comments (18)
src/components/IssueModal/CreateIssueModal/index.tsx (1)
247-247: Use a plain string literal forclassNamehere.Nice token migration to
bg-surface/50. Since there’s no interpolation, this can be a normal string to reduce noise.♻️ Suggested cleanup
- <div className={`-space-y-px overflow-hidden rounded-md bg-surface/50`}> + <div className="-space-y-px overflow-hidden rounded-md bg-surface/50">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/IssueModal/CreateIssueModal/index.tsx` at line 247, In the CreateIssueModal component, replace the unnecessary template literal used for the div's className (currently written as className={`-space-y-px overflow-hidden rounded-md bg-surface/50`}) with a plain string literal " -space-y-px overflow-hidden rounded-md bg-surface/50" (i.e., className="-space-y-px overflow-hidden rounded-md bg-surface/50") since there is no interpolation; locate the JSX div with that className and update it accordingly to reduce noise.src/components/Discover/Trending.tsx (1)
55-57: Migration to semantic tokens looks correct.Both icon-wrapper spans are now using the semantic
border-border-defaultandbg-inputtokens, aligning with the PR's theme token migration.Minor nit: the template literals (backticks) aren't needed since there's no interpolation—a plain string would suffice.
,
♻️ Optional: Use plain strings instead of template literals
- <span className={`inline-flex cursor-default items-center rounded-l-md border border-r-0 px-3 text-sm text-gray-100 border-border-default bg-input`}> + <span className="inline-flex cursor-default items-center rounded-l-md border border-r-0 px-3 text-sm text-gray-100 border-border-default bg-input">Apply similar change to line 77.
Also applies to: 77-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/Trending.tsx` around lines 55 - 57, In the Trending component, replace unnecessary template literal className values with plain strings for the icon-wrapper spans (the span containing CircleStackIcon and the other span around the second icon at lines ~77-79) — update className={`inline-flex cursor-default items-center rounded-l-md border border-r-0 px-3 text-sm text-gray-100 border-border-default bg-input`} and the similar occurrence to simply use "inline-flex cursor-default items-center rounded-l-md border border-r-0 px-3 text-sm text-gray-100 border-border-default bg-input" (i.e., remove backticks and curly braces) so there is no template literal when no interpolation is present.src/components/Layout/PullToRefresh/index.tsx (1)
120-120: Tokenize border/ring here for full theme consistency.
bg-surface-raisedis migrated, butborder-gray-800andring-gray-700remain hardcoded.Proposed class update
- } relative -top-28 h-9 w-9 rounded-full border-4 shadow-md shadow-black ring-1 border-gray-800 bg-surface-raised ring-gray-700`} + } relative -top-28 h-9 w-9 rounded-full border-4 shadow-md shadow-black ring-1 border-border-default bg-surface-raised ring-ring-app/20`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Layout/PullToRefresh/index.tsx` at line 120, The JSX in the PullToRefresh component (the element whose className contains "relative -top-28 h-9 w-9 rounded-full border-4 shadow-md shadow-black ring-1 border-gray-800 bg-surface-raised ring-gray-700") uses hardcoded color classes; replace border-gray-800 and ring-gray-700 with your theme token classes (e.g., the project’s tokenized border and ring utilities such as border-surface-stroke and ring-surface-outline or the equivalent token names in your design system) so the component uses tokenized colors consistently with bg-surface-raised; update the className string in the PullToRefresh render (the template literal containing those classes) to use the token names.src/components/Common/Modal/index.tsx (1)
119-119: Finish semantic token migration in modal chrome/backdrop.The modal container ring and backdrop gradient still use fixed grays (
ring-gray-700,from-gray-800/75,to-gray-800), which weakens theme fidelity for AMOLED-Strix.Proposed tweak
-className={`hide-scrollbar relative inline-block w-full overflow-auto px-4 pb-4 pt-4 text-left align-bottom shadow-xl ring-1 transition-all sm:my-8 sm:max-w-3xl sm:rounded-lg sm:align-middle bg-surface ring-gray-700 ${dialogClass}`} +className={`hide-scrollbar relative inline-block w-full overflow-auto px-4 pb-4 pt-4 text-left align-bottom bg-surface shadow-xl ring-1 ring-ring-app/10 transition-all sm:my-8 sm:max-w-3xl sm:rounded-lg sm:align-middle ${dialogClass}`} -<div className={`absolute inset-0 bg-gradient-to-b from-gray-800/75 to-gray-800`} /> +<div className="absolute inset-0 bg-gradient-to-b from-surface-raised/75 to-surface" />Also applies to: 147-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/Modal/index.tsx` at line 119, The modal component still uses hardcoded gray tokens in the container and backdrop (see the className on the modal container that includes "ring-gray-700" and the backdrop gradient classes referencing "from-gray-800/75" and "to-gray-800"); replace those fixed color classes with the semantic token equivalents used by the theme (e.g., use the project's semantic ring/backdrop tokens instead of ring-gray-700, from-gray-800/75, to-gray-800) so the modal respects AMOLED-Strix theming—update the className string on the modal container and the backdrop gradient construction (references: dialogClass and the modal backdrop element) to use the appropriate semantic token classes.src/components/Common/SlideOver/index.tsx (1)
74-74: Use semantic ring token for theme consistency.
bg-surfaceis migrated, butring-gray-700is still hard-coded and can look off under AMOLED-Strix. Prefer the semantic ring token here too.Proposed tweak
-<div className={`flex h-full flex-col rounded-lg shadow-xl ring-1 backdrop-blur bg-surface ring-gray-700`}> +<div className="flex h-full flex-col rounded-lg bg-surface shadow-xl ring-1 ring-ring-app/10 backdrop-blur">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/SlideOver/index.tsx` at line 74, The hard-coded color token "ring-gray-700" in the SlideOver component's container div should be replaced with your design system's semantic ring token to ensure theme consistency; update the className on the div in src/components/Common/SlideOver/index.tsx (the line containing the JSX: div className={`flex h-full flex-col rounded-lg shadow-xl ring-1 backdrop-blur bg-surface ring-gray-700`}) to use the semantic token (for example replace ring-gray-700 with your semantic token such as ring-border or ring-ring) so the ring color follows theming across themes.src/components/PersonDetails/index.tsx (1)
245-266: Deduplicate backdrop image derivation for both faders.The same filter/map/slice pipeline is repeated twice. Extract it once (memoized) and reuse it in both
ImageFaderinstances.♻️ Suggested refactor
+ const backdropImages = useMemo( + () => + [...(sortedCast ?? []), ...(sortedCrew ?? [])] + .filter((media) => media.backdropPath) + .map( + (media) => + `https://image.tmdb.org/t/p/w1920_and_h800_multi_faces/${media.backdropPath}` + ) + .slice(0, 6), + [sortedCast, sortedCrew] + ); ... - {(sortedCrew || sortedCast) && ( + {backdropImages.length > 0 && ( <> <div className="person-fader-standard absolute left-0 right-0 top-0 z-0 h-96"> - <ImageFader - isDarker - backgroundImages={[...(sortedCast ?? []), ...(sortedCrew ?? [])] - .filter((media) => media.backdropPath) - .map( - (media) => - `https://image.tmdb.org/t/p/w1920_and_h800_multi_faces/${media.backdropPath}` - ) - .slice(0, 6)} - /> + <ImageFader isDarker backgroundImages={backdropImages} /> </div> <div className="person-fader-amoled absolute left-0 right-0 top-0 z-0 h-96 overflow-hidden"> - <ImageFader - isDarker - backgroundImages={[...(sortedCast ?? []), ...(sortedCrew ?? [])] - .filter((media) => media.backdropPath) - .map( - (media) => - `https://image.tmdb.org/t/p/w1920_and_h800_multi_faces/${media.backdropPath}` - ) - .slice(0, 6)} - /> + <ImageFader isDarker backgroundImages={backdropImages} /> <div className="absolute inset-0 bg-gradient-to-b from-black/60 to-gray-900" /> </div> </> )}Also applies to: 257-269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/PersonDetails/index.tsx` around lines 245 - 266, Extract the repeated backdrop URL pipeline into a memoized value (e.g., const backdropUrls = useMemo(() => ..., [sortedCast, sortedCrew])) and replace the two inline arrays passed to ImageFader with that variable; specifically compute backdropUrls by concatenating [...(sortedCast ?? []), ...(sortedCrew ?? [])], filtering for media.backdropPath, mapping to the TMDB URL string, and slicing(0, 6), then pass backdropUrls to both ImageFader instances to remove duplication while depending on sortedCast and sortedCrew.src/components/CollectionDetails/index.tsx (1)
203-214: Letcollection-backdrop-overlayown the full gradient.This element still hard-codes
from-gray-900/50 to-gray-900, so the backdrop tint is partly anchored to the default palette even after moving overlay logic intocollection-backdrop-overlay. Move the full gradient into the CSS class, or switch these utilities to theme-aware tokens.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/CollectionDetails/index.tsx` around lines 203 - 214, The overlay currently hard-codes Tailwind utility gradient colors in the JSX; update the JSX in CollectionDetails (the element with className "collection-backdrop-overlay") to remove the inline utilities "bg-gradient-to-b from-gray-900/50 to-gray-900" so the element only references the CSS class, then move the full gradient definition into the stylesheet (or replace the utilities with theme-aware tokens) for ".collection-backdrop-overlay" so the backdrop tint is controlled by theme-aware CSS rather than hard-coded palette utilities.src/components/Common/ImageFader/index.tsx (1)
26-27: Avoid hard-coded theme id strings in component logic.Comparing against
'amoled-strix'directly can drift from the central theme definitions; prefer a sharedThemeId/THEMESconstant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/ImageFader/index.tsx` around lines 26 - 27, Replace the hard-coded string comparison in ImageFader (const { theme } = useTheme(); const isAmoled = theme === 'amoled-strix') with a reference to the centralized theme identifier constant (e.g., ThemeId.AMOLED_STRIX or THEMES.AMOLED_STRIX): import the appropriate constant from the shared theme definitions and use theme === <THEME_CONST> to set isAmoled so the component stays in sync with central theme enums/IDs.src/components/Common/Tooltip/index.tsx (1)
28-28: Use semantic border token in tooltip styling.
border-gray-600bypasses the new theme token system and can drift between themes.Proposed change
- `z-50 text-sm absolute font-normal px-2 py-1 rounded border shadow text-gray-100 bg-surface-raised border-gray-600`, + `z-50 text-sm absolute font-normal px-2 py-1 rounded border shadow text-gray-100 bg-surface-raised border-border-default`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/Tooltip/index.tsx` at line 28, The tooltip CSS currently uses a hardcoded class "border-gray-600" in the Tooltip component (src/components/Common/Tooltip/index.tsx) which bypasses theme tokens; update the class string in the Tooltip (the line that contains `z-50 text-sm absolute font-normal px-2 py-1 rounded border shadow text-gray-100 bg-surface-raised border-gray-600`) to use the semantic border token used by the design system (replace `border-gray-600` with the appropriate theme token, e.g. `border-surface-border` or `border-border`) so the tooltip border color follows theme tokens across themes.src/components/Toast/index.tsx (1)
30-30: Prefer semantic ring token for theme consistency.
ring-gray-500hard-codes a palette value and weakens AMOLED/default token remapping.Proposed change
- <div className={`pointer-events-auto w-full max-w-sm rounded-lg shadow-lg ring-1 bg-surface ring-gray-500`}> + <div className={`pointer-events-auto w-full max-w-sm rounded-lg shadow-lg ring-1 bg-surface ring-ring-app`}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Toast/index.tsx` at line 30, In the Toast component replace the hard-coded palette class "ring-gray-500" on the outer div (the div with className `pointer-events-auto w-full max-w-sm rounded-lg shadow-lg ring-1 bg-surface ...`) with the semantic ring token used across the design system (for example "ring-outline" or the project's equivalent), i.e., update the class string inside the Toast component to use the semantic token instead of "ring-gray-500" so theme tokens and AMOLED remapping apply consistently.src/components/RequestCard/index.tsx (1)
53-53: Consider migrating placeholder and error states to semantic tokens.
RequestCardPlaceholder(line 53) usesbg-gray-700andRequestCardError(line 85) usesbg-gray-800, while the mainRequestCardwas migrated tobg-surface. For consistent theming, consider updating these as well.♻️ Suggested changes
// Line 53 - RequestCardPlaceholder - <div className="relative w-72 animate-pulse rounded-xl bg-gray-700 p-4 sm:w-96"> + <div className="relative w-72 animate-pulse rounded-xl bg-surface p-4 sm:w-96"> // Line 85 - RequestCardError (keep ring-red-500 for error indication) - className="relative flex w-72 overflow-hidden rounded-xl bg-gray-800 p-4 text-gray-400 shadow ring-1 ring-red-500 sm:w-96" + className="relative flex w-72 overflow-hidden rounded-xl bg-surface p-4 text-gray-400 shadow ring-1 ring-red-500 sm:w-96"Also applies to: 85-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RequestCard/index.tsx` at line 53, RequestCardPlaceholder and RequestCardError use hardcoded tailwind classes bg-gray-700 and bg-gray-800; change them to use the semantic token class used by RequestCard (e.g., bg-surface or the project's semantic token like bg-surface-500) so theming is consistent. Locate the RequestCardPlaceholder component (the div with className "relative w-72 animate-pulse rounded-xl bg-gray-700 p-4 sm:w-96") and RequestCardError (the element using "bg-gray-800") and replace those bg-* classes with the semantic token class used by RequestCard (and update any pulse/spacing classes unchanged), ensuring both placeholders and error states follow the same semantic background token.src/components/Blocklist/index.tsx (1)
116-117: Consider migrating filter controls to semantic tokens for consistency.The filter and search icon holders (lines 116-117, 147-148) still use hardcoded
border-gray-500 bg-gray-800, while similar elements inRequestListwere migrated toborder-border-default bg-input. For visual consistency across themes, consider updating these as well.♻️ Suggested change for filter icon holder (line 116)
- <span className="inline-flex cursor-default items-center rounded-l-md border border-r-0 border-gray-500 bg-gray-800 px-3 text-sm text-gray-100"> + <span className="inline-flex cursor-default items-center rounded-l-md border border-r-0 border-border-default bg-input px-3 text-sm text-gray-100">Also applies to: 147-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Blocklist/index.tsx` around lines 116 - 117, The filter/search icon holders in src/components/Blocklist (the span wrapping <FunnelIcon> and the similar span for the search icon) still use hardcoded classes "border-gray-500 bg-gray-800"; update those className values to use the semantic tokens used in RequestList (e.g., replace "border-gray-500 bg-gray-800" with "border-border-default bg-input") so the components follow theme tokens and remain consistent across dark/light themes; ensure you update both occurrences (the FunnelIcon holder and the search icon holder) in the Blocklist component.src/components/RequestList/index.tsx (1)
276-276: Consider migrating the hover state to a semantic token.The hover state
hover:!bg-gray-400still uses a hardcoded gray color while other colors in this element use semantic tokens. For full theme consistency, consider using a semantic hover token if one is available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RequestList/index.tsx` at line 276, The hover style in the className on the RequestList component uses a hardcoded color token ("hover:!bg-gray-400"); update that to use the project's semantic token instead (replace "hover:!bg-gray-400" with the appropriate semantic class/variable used elsewhere, e.g., "hover:!bg-surface-hover" or a CSS variable like "hover:!bg-[var(--bg-hover)]") so the element follows theme tokens; edit the className string in src/components/RequestList/index.tsx where the class includes "z-40 mr-2 rounded-l-none border border-border-default !bg-input !px-3 !text-gray-500 hover:!bg-gray-400 hover:!text-white" and swap the hardcoded hover token for the project's semantic hover token.src/components/Discover/DiscoverTv/index.tsx (1)
70-72: Redundant type assertion.The
titlesarray is already typed asTvResult[]fromuseDiscover<TvResult, ...>, so the cast(t as TvResult)on lines 70 and 72 is unnecessary.♻️ Proposed simplification
-const withBackdrop = titles.filter((t) => (t as TvResult).backdropPath); +const withBackdrop = titles.filter((t) => t.backdropPath); if (!withBackdrop.length) return; -const pick = withBackdrop[Math.floor(Math.random() * withBackdrop.length)] as TvResult; +const pick = withBackdrop[Math.floor(Math.random() * withBackdrop.length)];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/DiscoverTv/index.tsx` around lines 70 - 72, The filter and pick logic redundantly cast each item to TvResult even though titles is already TvResult[] from useDiscover; remove the unnecessary casts by changing the filter to const withBackdrop = titles.filter(t => t.backdropPath); and keep pick as const pick = withBackdrop[Math.floor(Math.random() * withBackdrop.length)] as TvResult (or drop the cast there too since withBackdrop is TvResult[]), ensuring you reference the withBackdrop, titles and TvResult identifiers when making the change.src/components/Discover/DiscoverMovies/index.tsx (1)
70-77: Consider extracting shared backdrop logic.The backdrop selection logic here is identical to DiscoverTv. While not critical, extracting this to a shared hook would reduce duplication and ensure consistent behavior.
Example hook signature:
function useRandomBackdrop<T extends { backdropPath?: string }>(items: T[] | undefined): string | null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/DiscoverMovies/index.tsx` around lines 70 - 77, The backdrop selection useEffect in DiscoverMovies duplicates the same logic used in DiscoverTv; extract it into a shared hook like useRandomBackdrop<T extends { backdropPath?: string }>(items: T[] | undefined): string | null and replace the effect: move filtering for items with backdropPath, random pick, and pickedRef handling into the hook and return the chosen backdrop; then in this file remove the useEffect and pickedRef usage and call the hook to get the backdrop value and pass it to setBackdropPath (or directly use the returned value), keeping references to useEffect, pickedRef, setBackdropPath, and MovieResult to locate the original logic and ensure both DiscoverMovies and DiscoverTv consume the new hook for consistent behavior.src/components/Discover/HeroSlider/index.tsx (1)
75-80: Potential interval churn on slide changes.
startIntervalis recreated whennextchanges (which depends oncurrentIndex). SincestartIntervalis in the dependency array, the interval is cleared and recreated on every slide transition. This is likely benign but could cause subtle timing issues.♻️ Consider stabilizing with useRef
+const nextRef = useRef(next); +useEffect(() => { nextRef.current = next; }, [next]); + const startInterval = useCallback(() => { if (intervalRef.current) clearInterval(intervalRef.current); intervalRef.current = setInterval(() => { - if (!isPaused.current) next(); + if (!isPaused.current) nextRef.current(); }, ROTATE_INTERVAL); -}, [next]); +}, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/HeroSlider/index.tsx` around lines 75 - 80, The effect is recreating the interval whenever startInterval changes (because startInterval captures next/currentIndex), causing interval churn; stabilize the timer by removing the changing callback from the effect deps: either memoize startInterval with useCallback so it doesn't re-create on every next/currentIndex change or keep a stable wrapper in a ref (e.g., startIntervalRef) that invokes the latest next via a mutable ref; update useEffect to depend only on items.length and intervalRef, and call the stable startInterval (from the useCallback or ref) so intervals are not cleared/recreated on each slide transition.src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx (1)
47-55: Consider adding accessibility attributes to theme selection buttons.Screen reader users won't know which theme each button represents. Adding
aria-labeloraria-pressedwould improve accessibility.♿ Proposed accessibility improvement
<button key={t.id} onClick={() => setTheme(t.id)} + aria-label={`Select ${t.name} theme`} + aria-pressed={isActive} className={`relative flex flex-col overflow-hidden rounded-xl border-2 text-left transition duration-200 focus:outline-none ${🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx` around lines 47 - 55, The theme selection buttons in UserAppearanceSettings lack accessibility attributes; update the button (the element rendering per theme with key={t.id} and onClick={() => setTheme(t.id)}) to include an accessible label (e.g., aria-label using the theme name like t.name or t.label) and expose selection state via aria-pressed={isActive} (or aria-current when appropriate) so screen readers announce which theme each button represents and which one is active; keep existing visual styling and state logic (isActive, setTheme, bgColor) and only add these ARIA attributes to the button element.src/components/Common/Dropdown/index.tsx (1)
60-70: Inconsistent surface token usage for ghost variant.Line 64 uses
bg-surface-raised/90for the ghost variant, which is a semantic token. However, line 67 for glass uses hardcodedbg-black/80. Consider using semantic tokens consistently, or document when hardcoded values are intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/Dropdown/index.tsx` around lines 60 - 70, The ghost dropdown variant uses the semantic token bg-surface-raised/90 but the glass variant uses a hardcoded bg-black/80; update the Menu.Items className construction (the block referencing dropdownType, Menu.Items, dropdownType === 'ghost' and dropdownType === 'glass') to use a consistent semantic token for the glass variant (or a documented semantic token name) instead of bg-black/80, e.g., replace the hardcoded color with the appropriate semantic token (or add a comment documenting the intentional hardcoded value) and ensure className merging still includes the provided className and props.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 129-151: In the "Configure target registries" step, normalize GHCR
image names to lowercase by using the lowercase form of GITHUB_REPOSITORY when
composing primary_image and the images list; update the echo statements that set
primary_image and append ghcr.io/${GITHUB_REPOSITORY} (used in the images<<EOF
block and the primary_image assignment) to use the lowercase expansion (e.g.,
${GITHUB_REPOSITORY,,}) so all ghcr.io image references are consistently
lowercased.
In `@src/components/Common/Tag/index.tsx`:
- Line 11: The tag component's root div in Tag (the inline-flex chip) uses fixed
palette classes ring-gray-600 and hover:bg-gray-700; replace those with
theme-aware semantic token classes (e.g., use your design system's
surface-border / surface-hover tokens such as ring-surface-border and
hover:bg-surface-hover or the equivalent CSS variable tokens) so the ring and
hover states follow the current surface/theme; update the class string on the
div in src/components/Common/Tag/index.tsx accordingly to use those semantic
token names instead of the gray-600/gray-700 classes.
In `@src/components/Discover/HeroSlider/index.tsx`:
- Around line 239-244: Replace the hardcoded "View Details" label with the i18n
message used elsewhere: add a message key (e.g. viewDetails) to the messages
object for components.Discover.HeroSlider (where defineMessages is used) and
then use intl.formatMessage(messages.viewDetails) inside the Link component that
renders the button (the Link with href={detailUrl}) so the label is localized.
In `@src/components/Layout/AmoledNavbar/index.tsx`:
- Around line 185-186: The conditional is comparing
user?.displayName?.toLowerCase() to user?.email without normalizing both sides,
causing inconsistent case-sensitive behavior; update the JSX conditional (the
expression that currently uses user?.displayName and user?.email) to compare
normalized values for both fields (e.g., compare (user?.displayName ||
'').trim().toLowerCase() !== (user?.email || '').trim().toLowerCase()), ensuring
you guard against undefined/null before calling toLowerCase().
- Around line 70-73: The logout function (logout) currently calls
axios.post('/api/v1/auth/logout') and revalidate() with no error handling; wrap
the call in a try/catch so network or server errors are caught, call
revalidate() only on successful response.data?.status === 'ok', and in the catch
block log the error (console.error or processLogger) and surface user feedback
(e.g., show an error toast or set an error state) so the failure doesn't
silently propagate; ensure the function still returns/handles promise resolves
correctly to avoid unhandled rejections.
- Around line 50-57: The effect in AmoledNavbar (useEffect that focuses
searchInputRef and calls clear() and setIsOpen()) is missing dependencies:
include clear and setIsOpen in the dependency array or memoize clear inside
useSearchInput; either add clear and setIsOpen to the useEffect deps
([searchExpanded, isDesktop, clear, setIsOpen]) or change the clear function in
useSearchInput to be stable using useCallback (exporting the stable clear) so
the existing dependency array stays valid.
In `@src/components/MediaSlider/index.tsx`:
- Around line 180-196: The "See all" label is hardcoded; update the MediaSlider
header to use intl.formatMessage for i18n: add a message id (e.g.
"mediaSlider.seeAll") to your messages bundle and replace the literal "See all"
inside the Link (the JSX that renders when linkUrl is truthy) with
intl.formatMessage({ id: 'mediaSlider.seeAll' }) — ensure the component imports
and calls useIntl() (or uses the existing intl) and keep the surrounding
elements (Link, ChevronRightIcon, title, linkUrl) unchanged.
In `@src/components/MovieDetails/index.tsx`:
- Line 1411: The four hardcoded section headings ("Crew", "Keywords", "Ratings",
"Details") in MovieDetails (the h3 elements with className "text-xs
font-semibold uppercase tracking-wider text-white/30") must be moved into the
component's messages and rendered via the intl API (e.g., intl.formatMessage or
<FormattedMessage>) instead of literal strings; add keys to the messages object
(e.g., crewLabel, keywordsLabel, ratingsLabel, detailsLabel), import/consume the
intl instance used elsewhere in this file, and replace the literal text in the
h3 elements with the localized message lookups so the AMOLED-only branch
translates correctly.
- Around line 434-436: The JSX return in MovieDetails is broken by a stray
closing ternary branch ") : (" without a matching opening condition; restore a
proper conditional around the two layout branches (e.g., replace the initial
fragment opening with a condition like someFlag ? ( ... ) : ( ... ) or delete
one branch so only one layout remains) by editing the MovieDetails component's
return block. Also in the AMOLED branch replace the hardcoded English headers
"Crew", "Keywords", "Ratings", and "Details" with localized calls using
intl.formatMessage (for example intl.formatMessage({ id: 'movie.crew',
defaultMessage: 'Crew' }) and similarly for keywords, ratings, details) so those
labels use the app i18n.
In `@src/components/Settings/OverrideRule/OverrideRuleTiles.tsx`:
- Around line 142-143: The list items rendered inside the rules.map in the
OverrideRuleTiles component lack a stable React key; update the JSX in the map
(the <li> produced for each rule in rules.map) to include a stable key prop
(preferably rule.id or another unique identifier on the rule object, falling
back to a composite stable string like `${rule.id || rule.name}` and only use
index as last resort) so React can correctly track rule tiles across
edits/deletes.
In `@src/components/Slider/index.tsx`:
- Around line 153-171: The two icon-only navigation buttons (the left/right
buttons that call slide(Direction.LEFT) and slide(Direction.RIGHT) and render
ChevronLeftIcon/ChevronRightIcon) lack accessible names; add accessible labels
by providing either an aria-label (e.g., aria-label="Previous slide" and
aria-label="Next slide") or include visually-hidden text inside the button so
screen readers can identify them, and ensure the attributes remain when disabled
(do not remove or conditionally hide the label based on scrollPos.isStart or
scrollPos.isEnd).
In `@src/components/TitleCard/index.tsx`:
- Around line 500-513: Duplicate data-testid "title-card-title" is used for both
AMOLED and standard title blocks in the TitleCard component; update the test ids
to be unique by changing the AMOLED block's data-testid (identified by the
container className "title-card-amoled-title-block" and the h1 using className
"mb-1 whitespace-normal text-sm font-bold leading-tight text-white") to
something like "title-card-title-amoled" and change the standard title block's
h1 to "title-card-title-standard" (or similar unique names) so tests can target
each element unambiguously.
- Around line 377-383: The icon-only AMOLED action buttons (e.g., the <button>
wrapping <StarIcon> and the <Button> with buttonType={'ghost'}) lack accessible
names; update those elements and the similar remove-watchlist and blocklist icon
buttons (lines referenced around the StarIcon and Button usage) to include
type="button" and an appropriate aria-label (e.g., aria-label="Add to watchlist"
/ "Remove from watchlist" / "Block user") so screen readers can announce their
purpose, keeping their existing onClick handlers like onClickWatchlistBtn
intact; ensure you add aria-label to both native <button> elements and the
custom <Button> component props if it forwards attributes.
In `@src/components/TvDetails/index.tsx`:
- Line 1670: The headings "Crew", "Keywords", "Ratings", and "Details" are
hardcoded in the TvDetails component and must be localized; replace the literal
strings in the JSX (the h3 elements that render "Crew", "Keywords", "Ratings",
"Details") with calls to intl.formatMessage using new message entries (e.g.,
tvDetails.crew, tvDetails.keywords, tvDetails.ratings, tvDetails.details) and
add those IDs to your messages file; ensure you import/use the existing intl
instance in this component and render intl.formatMessage({ id: 'tvDetails.crew'
}) (and the corresponding IDs for the other three headings) where the current
text nodes are.
- Around line 465-467: The returned JSX currently contains a broken branch
switch (a stray ") : (" without a matching "?") between the standard and AMOLED
layout branches inside the fragment that begins with the div having className
"tv-details-backdrop-section media-page"; fix this by wrapping the two layout
branches in a proper ternary expression (e.g., isAmoled ? (/* AMOLED branch */)
: (/* standard branch */)) so the fragment parses correctly and remove the stray
separator comment that Biome flagged; additionally replace the hardcoded English
labels "Crew", "Ratings", and "Details" in the AMOLED branch with react-intl
usage (useFormattedMessage or <FormattedMessage id="tv.crew"/>, id="tv.ratings",
id="tv.details" or the project's equivalent intl.formatMessage calls) so
localization is preserved.
In `@src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx`:
- Around line 43-56: The code destructures t.swatches inside THEMES.map (const
[bgColor, surfaceColor, accentColor] = t.swatches) but doesn't guard against
shorter arrays, causing undefined colors to be applied; fix by adding a
defensive check/fallback: when rendering each theme in the THEMES.map callback
(where isActive, setTheme, theme are used), ensure t.swatches exists and has at
least three entries and supply sensible default values (or fallback the single
bgColor used in style to a default like 'transparent') so style={{ background:
... }} never receives undefined; update the destructuring or the style
expression to use these fallback values (referencing t.swatches, bgColor,
surfaceColor, accentColor in that component).
In `@src/context/ThemeContext.tsx`:
- Around line 42-49: Read localStorage synchronously before React mounts and use
that value as the initial state so the saved theme is applied before first
paint: implement a module-level resolver that reads STORAGE_KEY and validates
against THEMES (falling back to DEFAULT_THEME), use that resolved value as the
initial argument to useState (instead of DEFAULT_THEME) in ThemeContext (so
setThemeState starts with the persisted theme), and ensure you still set
document.documentElement.setAttribute('data-theme', resolved) immediately after
resolving so the <html> attribute is updated before hydration; keep references
to STORAGE_KEY, THEMES, DEFAULT_THEME, setThemeState, and
document.documentElement.setAttribute when making the change.
- Around line 13-25: THEMES currently embeds hardcoded English display strings
(name/description) which breaks localization; update the ThemeDefinition entries
to only include stable identifiers (keep id and swatches) or add explicit
translation keys (e.g. nameKey/descriptionKey) instead of English text, then
change any UI that reads THEMES (the appearance picker / theme renderer) to call
the i18n translation function with those keys (for example
i18n.t(`themes.${id}.name`) and i18n.t(`themes.${id}.description`)) so
translations are provided at render time; ensure ThemeDefinition type is updated
accordingly and replace uses of name/description with the translated strings.
In `@src/styles/globals.css`:
- Around line 653-659: The gradient uses Tailwind's theme('colors.gray.900')
which isn't valid in plain CSS; update the rule targeting
[data-theme='amoled-strix'] .collection-backdrop-overlay and
.issue-backdrop-overlay to use a concrete color value or CSS variable (e.g.,
replace theme('colors.gray.900') with the hex for gray-900 or with
var(--color-gray-900) / var(--tw-gradient-to) if you provide that variable) so
the linear-gradient resolves correctly in runtime.
---
Outside diff comments:
In `@src/components/Login/JellyfinLogin.tsx`:
- Around line 124-130: The background utility on the input Fields is being
overridden by the input[type='text'] / input[type='password'] selector, so
restore the important flag by changing the className on the Field with
id="username" (and the corresponding Field with id="password") in the
JellyfinLogin component to use "!bg-input" instead of "bg-input" (keep the other
classes like "placeholder:text-gray-400"); this mirrors the existing fix used in
RequestList.tsx and ensures the intended background class wins specificity.
In `@src/pages/_app.tsx`:
- Around line 203-223: The app flashes the wrong theme because ThemeProvider
only sets the data-theme inside a client-side useEffect; to fix, add an inline
script in the custom document that runs before paint and sets
document.documentElement.setAttribute('data-theme', <value>) by reading the same
localStorage key your ThemeProvider uses (or alternatively pass the persisted
theme from SSR into currentSettings and render it on the server); target the
ThemeProvider/data-theme behavior and ensure the inline script reads the
identical storage key and sets the attribute synchronously so the correct theme
is present on first paint.
---
Nitpick comments:
In `@src/components/Blocklist/index.tsx`:
- Around line 116-117: The filter/search icon holders in
src/components/Blocklist (the span wrapping <FunnelIcon> and the similar span
for the search icon) still use hardcoded classes "border-gray-500 bg-gray-800";
update those className values to use the semantic tokens used in RequestList
(e.g., replace "border-gray-500 bg-gray-800" with "border-border-default
bg-input") so the components follow theme tokens and remain consistent across
dark/light themes; ensure you update both occurrences (the FunnelIcon holder and
the search icon holder) in the Blocklist component.
In `@src/components/CollectionDetails/index.tsx`:
- Around line 203-214: The overlay currently hard-codes Tailwind utility
gradient colors in the JSX; update the JSX in CollectionDetails (the element
with className "collection-backdrop-overlay") to remove the inline utilities
"bg-gradient-to-b from-gray-900/50 to-gray-900" so the element only references
the CSS class, then move the full gradient definition into the stylesheet (or
replace the utilities with theme-aware tokens) for
".collection-backdrop-overlay" so the backdrop tint is controlled by theme-aware
CSS rather than hard-coded palette utilities.
In `@src/components/Common/Dropdown/index.tsx`:
- Around line 60-70: The ghost dropdown variant uses the semantic token
bg-surface-raised/90 but the glass variant uses a hardcoded bg-black/80; update
the Menu.Items className construction (the block referencing dropdownType,
Menu.Items, dropdownType === 'ghost' and dropdownType === 'glass') to use a
consistent semantic token for the glass variant (or a documented semantic token
name) instead of bg-black/80, e.g., replace the hardcoded color with the
appropriate semantic token (or add a comment documenting the intentional
hardcoded value) and ensure className merging still includes the provided
className and props.
In `@src/components/Common/ImageFader/index.tsx`:
- Around line 26-27: Replace the hard-coded string comparison in ImageFader
(const { theme } = useTheme(); const isAmoled = theme === 'amoled-strix') with a
reference to the centralized theme identifier constant (e.g.,
ThemeId.AMOLED_STRIX or THEMES.AMOLED_STRIX): import the appropriate constant
from the shared theme definitions and use theme === <THEME_CONST> to set
isAmoled so the component stays in sync with central theme enums/IDs.
In `@src/components/Common/Modal/index.tsx`:
- Line 119: The modal component still uses hardcoded gray tokens in the
container and backdrop (see the className on the modal container that includes
"ring-gray-700" and the backdrop gradient classes referencing "from-gray-800/75"
and "to-gray-800"); replace those fixed color classes with the semantic token
equivalents used by the theme (e.g., use the project's semantic ring/backdrop
tokens instead of ring-gray-700, from-gray-800/75, to-gray-800) so the modal
respects AMOLED-Strix theming—update the className string on the modal container
and the backdrop gradient construction (references: dialogClass and the modal
backdrop element) to use the appropriate semantic token classes.
In `@src/components/Common/SlideOver/index.tsx`:
- Line 74: The hard-coded color token "ring-gray-700" in the SlideOver
component's container div should be replaced with your design system's semantic
ring token to ensure theme consistency; update the className on the div in
src/components/Common/SlideOver/index.tsx (the line containing the JSX: div
className={`flex h-full flex-col rounded-lg shadow-xl ring-1 backdrop-blur
bg-surface ring-gray-700`}) to use the semantic token (for example replace
ring-gray-700 with your semantic token such as ring-border or ring-ring) so the
ring color follows theming across themes.
In `@src/components/Common/Tooltip/index.tsx`:
- Line 28: The tooltip CSS currently uses a hardcoded class "border-gray-600" in
the Tooltip component (src/components/Common/Tooltip/index.tsx) which bypasses
theme tokens; update the class string in the Tooltip (the line that contains
`z-50 text-sm absolute font-normal px-2 py-1 rounded border shadow text-gray-100
bg-surface-raised border-gray-600`) to use the semantic border token used by the
design system (replace `border-gray-600` with the appropriate theme token, e.g.
`border-surface-border` or `border-border`) so the tooltip border color follows
theme tokens across themes.
In `@src/components/Discover/DiscoverMovies/index.tsx`:
- Around line 70-77: The backdrop selection useEffect in DiscoverMovies
duplicates the same logic used in DiscoverTv; extract it into a shared hook like
useRandomBackdrop<T extends { backdropPath?: string }>(items: T[] | undefined):
string | null and replace the effect: move filtering for items with
backdropPath, random pick, and pickedRef handling into the hook and return the
chosen backdrop; then in this file remove the useEffect and pickedRef usage and
call the hook to get the backdrop value and pass it to setBackdropPath (or
directly use the returned value), keeping references to useEffect, pickedRef,
setBackdropPath, and MovieResult to locate the original logic and ensure both
DiscoverMovies and DiscoverTv consume the new hook for consistent behavior.
In `@src/components/Discover/DiscoverTv/index.tsx`:
- Around line 70-72: The filter and pick logic redundantly cast each item to
TvResult even though titles is already TvResult[] from useDiscover; remove the
unnecessary casts by changing the filter to const withBackdrop = titles.filter(t
=> t.backdropPath); and keep pick as const pick =
withBackdrop[Math.floor(Math.random() * withBackdrop.length)] as TvResult (or
drop the cast there too since withBackdrop is TvResult[]), ensuring you
reference the withBackdrop, titles and TvResult identifiers when making the
change.
In `@src/components/Discover/HeroSlider/index.tsx`:
- Around line 75-80: The effect is recreating the interval whenever
startInterval changes (because startInterval captures next/currentIndex),
causing interval churn; stabilize the timer by removing the changing callback
from the effect deps: either memoize startInterval with useCallback so it
doesn't re-create on every next/currentIndex change or keep a stable wrapper in
a ref (e.g., startIntervalRef) that invokes the latest next via a mutable ref;
update useEffect to depend only on items.length and intervalRef, and call the
stable startInterval (from the useCallback or ref) so intervals are not
cleared/recreated on each slide transition.
In `@src/components/Discover/Trending.tsx`:
- Around line 55-57: In the Trending component, replace unnecessary template
literal className values with plain strings for the icon-wrapper spans (the span
containing CircleStackIcon and the other span around the second icon at lines
~77-79) — update className={`inline-flex cursor-default items-center
rounded-l-md border border-r-0 px-3 text-sm text-gray-100 border-border-default
bg-input`} and the similar occurrence to simply use "inline-flex cursor-default
items-center rounded-l-md border border-r-0 px-3 text-sm text-gray-100
border-border-default bg-input" (i.e., remove backticks and curly braces) so
there is no template literal when no interpolation is present.
In `@src/components/IssueModal/CreateIssueModal/index.tsx`:
- Line 247: In the CreateIssueModal component, replace the unnecessary template
literal used for the div's className (currently written as
className={`-space-y-px overflow-hidden rounded-md bg-surface/50`}) with a plain
string literal " -space-y-px overflow-hidden rounded-md bg-surface/50" (i.e.,
className="-space-y-px overflow-hidden rounded-md bg-surface/50") since there is
no interpolation; locate the JSX div with that className and update it
accordingly to reduce noise.
In `@src/components/Layout/PullToRefresh/index.tsx`:
- Line 120: The JSX in the PullToRefresh component (the element whose className
contains "relative -top-28 h-9 w-9 rounded-full border-4 shadow-md shadow-black
ring-1 border-gray-800 bg-surface-raised ring-gray-700") uses hardcoded color
classes; replace border-gray-800 and ring-gray-700 with your theme token classes
(e.g., the project’s tokenized border and ring utilities such as
border-surface-stroke and ring-surface-outline or the equivalent token names in
your design system) so the component uses tokenized colors consistently with
bg-surface-raised; update the className string in the PullToRefresh render (the
template literal containing those classes) to use the token names.
In `@src/components/PersonDetails/index.tsx`:
- Around line 245-266: Extract the repeated backdrop URL pipeline into a
memoized value (e.g., const backdropUrls = useMemo(() => ..., [sortedCast,
sortedCrew])) and replace the two inline arrays passed to ImageFader with that
variable; specifically compute backdropUrls by concatenating [...(sortedCast ??
[]), ...(sortedCrew ?? [])], filtering for media.backdropPath, mapping to the
TMDB URL string, and slicing(0, 6), then pass backdropUrls to both ImageFader
instances to remove duplication while depending on sortedCast and sortedCrew.
In `@src/components/RequestCard/index.tsx`:
- Line 53: RequestCardPlaceholder and RequestCardError use hardcoded tailwind
classes bg-gray-700 and bg-gray-800; change them to use the semantic token class
used by RequestCard (e.g., bg-surface or the project's semantic token like
bg-surface-500) so theming is consistent. Locate the RequestCardPlaceholder
component (the div with className "relative w-72 animate-pulse rounded-xl
bg-gray-700 p-4 sm:w-96") and RequestCardError (the element using "bg-gray-800")
and replace those bg-* classes with the semantic token class used by RequestCard
(and update any pulse/spacing classes unchanged), ensuring both placeholders and
error states follow the same semantic background token.
In `@src/components/RequestList/index.tsx`:
- Line 276: The hover style in the className on the RequestList component uses a
hardcoded color token ("hover:!bg-gray-400"); update that to use the project's
semantic token instead (replace "hover:!bg-gray-400" with the appropriate
semantic class/variable used elsewhere, e.g., "hover:!bg-surface-hover" or a CSS
variable like "hover:!bg-[var(--bg-hover)]") so the element follows theme
tokens; edit the className string in src/components/RequestList/index.tsx where
the class includes "z-40 mr-2 rounded-l-none border border-border-default
!bg-input !px-3 !text-gray-500 hover:!bg-gray-400 hover:!text-white" and swap
the hardcoded hover token for the project's semantic hover token.
In `@src/components/Toast/index.tsx`:
- Line 30: In the Toast component replace the hard-coded palette class
"ring-gray-500" on the outer div (the div with className `pointer-events-auto
w-full max-w-sm rounded-lg shadow-lg ring-1 bg-surface ...`) with the semantic
ring token used across the design system (for example "ring-outline" or the
project's equivalent), i.e., update the class string inside the Toast component
to use the semantic token instead of "ring-gray-500" so theme tokens and AMOLED
remapping apply consistently.
In `@src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx`:
- Around line 47-55: The theme selection buttons in UserAppearanceSettings lack
accessibility attributes; update the button (the element rendering per theme
with key={t.id} and onClick={() => setTheme(t.id)}) to include an accessible
label (e.g., aria-label using the theme name like t.name or t.label) and expose
selection state via aria-pressed={isActive} (or aria-current when appropriate)
so screen readers announce which theme each button represents and which one is
active; keep existing visual styling and state logic (isActive, setTheme,
bgColor) and only add these ARIA attributes to the button element.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89ba5e68-af40-4239-8af4-222cdc7d835a
📒 Files selected for processing (71)
.github/workflows/release.ymlsrc/components/Blocklist/index.tsxsrc/components/CollectionDetails/index.tsxsrc/components/Common/ButtonWithDropdown/index.tsxsrc/components/Common/Dropdown/index.tsxsrc/components/Common/ImageFader/index.tsxsrc/components/Common/Modal/index.tsxsrc/components/Common/PlayButton/index.tsxsrc/components/Common/SettingsTabs/index.tsxsrc/components/Common/SlideOver/index.tsxsrc/components/Common/Table/index.tsxsrc/components/Common/Tag/index.tsxsrc/components/Common/Tooltip/index.tsxsrc/components/Discover/DiscoverMovies/index.tsxsrc/components/Discover/DiscoverSliderEdit/index.tsxsrc/components/Discover/DiscoverTv/index.tsxsrc/components/Discover/HeroSlider/index.tsxsrc/components/Discover/Trending.tsxsrc/components/Discover/index.tsxsrc/components/IssueDetails/IssueComment/index.tsxsrc/components/IssueDetails/index.tsxsrc/components/IssueList/IssueItem/index.tsxsrc/components/IssueList/index.tsxsrc/components/IssueModal/CreateIssueModal/index.tsxsrc/components/Layout/AmoledNavbar/index.tsxsrc/components/Layout/PullToRefresh/index.tsxsrc/components/Layout/UserDropdown/index.tsxsrc/components/Layout/index.tsxsrc/components/Login/JellyfinLogin.tsxsrc/components/Login/LocalLogin.tsxsrc/components/Login/index.tsxsrc/components/MediaSlider/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/PersonDetails/index.tsxsrc/components/RegionSelector/index.tsxsrc/components/RequestBlock/index.tsxsrc/components/RequestButton/index.tsxsrc/components/RequestCard/index.tsxsrc/components/RequestList/RequestItem/index.tsxsrc/components/RequestList/index.tsxsrc/components/RequestModal/AdvancedRequester/index.tsxsrc/components/ResetPassword/RequestResetLink.tsxsrc/components/ResetPassword/index.tsxsrc/components/Settings/OverrideRule/OverrideRuleTiles.tsxsrc/components/Settings/SettingsAbout/Releases/index.tsxsrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsLogs/index.tsxsrc/components/Settings/SettingsMetadata.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/Settings/SettingsServices.tsxsrc/components/Setup/JellyfinSetup.tsxsrc/components/Setup/index.tsxsrc/components/Slider/index.tsxsrc/components/TitleCard/ErrorCard.tsxsrc/components/TitleCard/index.tsxsrc/components/Toast/index.tsxsrc/components/TvDetails/Season/index.tsxsrc/components/TvDetails/index.tsxsrc/components/UserList/index.tsxsrc/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsxsrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsWebPush/DeviceItem.tsxsrc/components/UserProfile/UserSettings/index.tsxsrc/components/UserProfile/index.tsxsrc/context/ThemeContext.tsxsrc/hooks/useTheme.tssrc/pages/_app.tsxsrc/pages/profile/settings/appearance.tsxsrc/pages/users/[userId]/settings/appearance.tsxsrc/styles/globals.csstailwind.config.js
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Settings/OverrideRule/OverrideRuleTiles.tsx (1)
155-163:⚠️ Potential issue | 🟡 MinorAdd
keyprops to inner map iterations.Multiple nested
.map()calls render elements without keys (e.g., lines 155-163 for users, 174-178 for genres, 191-201 for languages, 212-221 for keywords, 259-271 for tags). While less critical than the outer<li>, these should have stable keys for correct reconciliation.Example fix for users:
{rule.users.split(',').map((userId) => { return ( - <span> + <span key={userId}> { users?.find((user) => user.id === Number(userId)) ?.displayName } </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/OverrideRule/OverrideRuleTiles.tsx` around lines 155 - 163, The inner .map() iterations inside the OverrideRuleTiles component render spans without key props (e.g., the users map over rule.users, genres map, languages map, keywords map, and tags map), causing unstable reconciliation; update each mapped element (the span or fragment returned in the users.map, genres.map, languages.map, keywords.map, and tags.map callbacks) to include a stable key prop derived from the item (use userId or the mapped id/string such as Number(userId), genre id/name, language code/id, keyword id/text, or tag id) so React can track them reliably (ensure keys are unique within each list and use the parsed Number(userId) or the original identifier as appropriate).
♻️ Duplicate comments (10)
src/components/Common/Tag/index.tsx (1)
11-11:⚠️ Potential issue | 🟡 MinorUse semantic ring/hover tokens so Tag stays theme-consistent.
Line 11 still mixes
bg-surfacewith fixed palette classes (ring-gray-600,hover:bg-gray-700), so ring/hover won’t adapt to AMOLED-Strix.Suggested update
- <div className={`inline-flex cursor-pointer items-center rounded-full px-2 py-1 text-sm text-gray-200 ring-1 transition bg-surface ring-gray-600 hover:bg-gray-700`}> + <div className="inline-flex cursor-pointer items-center rounded-full bg-surface px-2 py-1 text-sm text-gray-200 ring-1 ring-ring-app transition hover:bg-surface-raised">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/Tag/index.tsx` at line 11, The Tag component's root div (in src/components/Common/Tag/index.tsx — the inline-flex cursor-pointer items-center rounded-full ... element) uses fixed palette classes (ring-gray-600 and hover:bg-gray-700) which break theming; replace those fixed classes with the project's semantic tokens (e.g., use the theme's ring and hover background tokens such as ring-<semantic-token> and hover:bg-<semantic-token> — e.g., ring-border/ ring-muted and hover:bg-surface-hover or the equivalent tokens your design system defines) so the ring and hover states adapt to AMOLED-Strix and other themes while keeping the rest of the class list intact.src/components/MediaSlider/index.tsx (1)
192-192:⚠️ Potential issue | 🟡 MinorLocalize hardcoded “See all” label.
Line 192 still uses a hardcoded user-facing string, so this won’t be translated in non-English locales.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MediaSlider/index.tsx` at line 192, Replace the hardcoded "See all" string in the MediaSlider component with a localized message: import and use your app's i18n helper (e.g., useTranslation from 'react-i18next' or the project's t() utility) inside the MediaSlider component and replace the literal "See all" JSX text with t('mediaSlider.seeAll') (or the project's chosen key); also add the 'mediaSlider.seeAll' entry to the locale resource files for supported languages so translations are available.src/components/Discover/HeroSlider/index.tsx (1)
239-244:⚠️ Potential issue | 🟡 MinorHardcoded "View Details" text should be internationalized.
The "View Details" button text is hardcoded in English while other UI text uses
intl.formatMessage. This breaks i18n consistency.🌐 Proposed fix
Add to messages definition:
import defineMessages from '@app/utils/defineMessages'; const messages = defineMessages('components.Discover.HeroSlider', { viewDetails: 'View Details', });Then use it:
-View Details +{intl.formatMessage(messages.viewDetails)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/HeroSlider/index.tsx` around lines 239 - 244, The "View Details" text in the Link inside the HeroSlider is hardcoded; add a message key (e.g., viewDetails) to the component's messages definition (use defineMessages with namespace 'components.Discover.HeroSlider') and replace the literal "View Details" with intl.formatMessage(messages.viewDetails) in the Link (the element using href={detailUrl}); this aligns with existing i18n usage in the component.src/components/MovieDetails/index.tsx (2)
1411-1411:⚠️ Potential issue | 🟡 MinorLocalize the AMOLED section headings.
Lines 1411, 1435, 1458, and 1496 hardcode "Crew", "Keywords", "Ratings", and "Details" in English. The standard layout uses
intl.formatMessage()for these labels; the AMOLED branch should do the same.🌐 Proposed fix
The existing
messagesobject already hascastbut not these labels. Add them:const messages = defineMessages('components.MovieDetails', { // ... existing messages crew: 'Crew', keywords: 'Keywords', ratings: 'Ratings', details: 'Details', });Then replace hardcoded strings:
-<h3 className="...">Crew</h3> +<h3 className="...">{intl.formatMessage(messages.crew)}</h3> -<h3 className="...">Keywords</h3> +<h3 className="...">{intl.formatMessage(messages.keywords)}</h3> -<h2 className="...">Ratings</h2> +<h2 className="...">{intl.formatMessage(messages.ratings)}</h2> -<h2 className="...">Details</h2> +<h2 className="...">{intl.formatMessage(messages.details)}</h2>Also applies to: 1435-1435, 1458-1458, 1496-1496
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` at line 1411, Add localized labels for the AMOLED headings by extending the existing messages object (identifier: messages) with keys crew, keywords, ratings, and details and then replace the hardcoded strings "Crew", "Keywords", "Ratings", and "Details" in the JSX with intl.formatMessage(messages.crew), intl.formatMessage(messages.keywords), intl.formatMessage(messages.ratings), and intl.formatMessage(messages.details) respectively; update usages near the h3 headings (the elements rendering those uppercase labels) so they pull from intl.formatMessage instead of literal strings.
1136-1139:⚠️ Potential issue | 🔴 CriticalCritical: Broken ternary operator - missing opening condition.
Line 1137 has
) : (which implies a ternary expression, but the return statement at line 434 opens with<>(a fragment), not a conditional. This creates invalid JSX syntax that will fail to compile.The structure appears to be:
return ( <> <div className="movie-details-backdrop-section ..."> {/* ~700 lines of standard layout */} </div> ) : ( // ← ERROR: No matching ternary opening <div className="relative bg-black"> {/* AMOLED layout */} </div> </> );You need to add the opening condition before the standard layout:
🐛 Proposed fix structure
return ( - <> - <div className="movie-details-backdrop-section ..."> + isAmoledTheme ? ( + <div className="relative bg-black"> + {/* AMOLED layout */} + </div> + ) : ( + <div className="movie-details-backdrop-section ..."> {/* Standard layout */} </div> - ) : ( - <div className="relative bg-black"> - {/* AMOLED layout */} - </div> - </> + ) );Or wrap with the theme check:
const { theme } = useTheme(); const isAmoled = theme === 'amoled-strix';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` around lines 1136 - 1139, The JSX contains a stray ternary closing token ") : (" in the MovieDetails component causing a syntax error; fix it by introducing a proper conditional around the two layout branches (standard vs AMOLED) — e.g., get theme from useTheme() (const { theme } = useTheme()) and compute isAmoled (theme === 'amoled-strix'), then replace the fragment-wrapped pair with a ternary expression using isAmoled ? <div className="relative bg-black">…</div> : <div className="movie-details-backdrop-section …">…</div> (or invert the order to match intended UX) so the opening condition matches the existing ") : (" and removes the stray token.src/styles/globals.css (1)
657-663:⚠️ Potential issue | 🟠 MajorInvalid use of
theme()function in vanilla CSS.The
theme('colors.gray.900')function on line 662 is a Tailwind directive that only works within@applyor Tailwind's processing context. In a standard CSS rule, this will output literally astheme('colors.gray.900')and fail to resolve.🐛 Proposed fix using hardcoded RGB value
[data-theme='amoled-strix'] .collection-backdrop-overlay, [data-theme='amoled-strix'] .issue-backdrop-overlay { --tw-gradient-from: rgba(0, 0, 0, 0.6); --tw-gradient-stops: var(--tw-gradient-from), var(--tw-gradient-to); - background-image: linear-gradient(to bottom, rgba(0, 0, 0, 0.6), theme('colors.gray.900')); + background-image: linear-gradient(to bottom, rgba(0, 0, 0, 0.6), rgb(17 24 39)); }Or use the CSS variable for theme-awareness:
- background-image: linear-gradient(to bottom, rgba(0, 0, 0, 0.6), theme('colors.gray.900')); + background-image: linear-gradient(to bottom, rgba(0, 0, 0, 0.6), rgb(var(--gray-900)));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/styles/globals.css` around lines 657 - 663, The CSS rule for the selectors .collection-backdrop-overlay and .issue-backdrop-overlay uses Tailwind's theme('colors.gray.900') which won't be resolved in plain CSS; replace that call with a concrete color value or a CSS variable (for example the equivalent rgba/hex for gray.900 or var(--color-gray-900)) and ensure --tw-gradient-to is set if you want theme-aware behavior so the background-image uses a valid color instead of theme('colors.gray.900').src/components/TvDetails/index.tsx (2)
360-362:⚠️ Potential issue | 🔴 CriticalCritical syntax error: broken ternary expression.
The code at line 1360 has
) : (without a corresponding?conditional operator. The fragment opens at line 465 with<>, the standard layout ends at line 1359, then line 1360 attempts to switch branches with) : (, but there's no ternary condition. This will cause a parse error and the component won't compile.Additionally, line 1361 has a bare comment that Biome flags - JSX comments must be wrapped in braces:
{/* AMOLED full-bleed layout */}.You need to introduce a conditional (e.g.,
isAmoled ? (...) : (...)) to properly switch between the standard and AMOLED layouts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` around lines 360 - 362, The JSX currently has a broken ternary in the TvDetails component: replace the stray `) : (` branch switch with a proper conditional using a boolean (e.g., isAmoled) so the component renders either the standard layout or the AMOLED layout (turn the fragment split into isAmoled ? (standard JSX) : (amoled JSX)); ensure the boolean isAmoled is defined/derived in TvDetails (or passed as a prop) and use it in the ternary, and convert the bare comment "AMOLED full-bleed layout" into a JSX comment wrapped in braces `{/* AMOLED full-bleed layout */}` so the file parses correctly.
1670-1670:⚠️ Potential issue | 🟡 MinorLocalize the AMOLED section headings.
The AMOLED branch hardcodes English strings for section headings: "Crew" (line 1670), "Keywords" (line 1708), "Ratings" (line 1730), and "Details" (line 1775). These bypass
react-intland will not be translated.Add message definitions and use
intl.formatMessage():-<h3 className="text-xs font-semibold uppercase tracking-wider text-white/30">Crew</h3> +<h3 className="text-xs font-semibold uppercase tracking-wider text-white/30">{intl.formatMessage(messages.crew)}</h3>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` at line 1670, The hardcoded section headings ("Crew", "Keywords", "Ratings", "Details") in the TvDetails component should be replaced with localized messages; add message identifiers (e.g. tvDetails.crew/tvDetails.keywords/tvDetails.ratings/tvDetails.details) to the messages file and use intl.formatMessage({ id: 'tvDetails.crew', defaultMessage: 'Crew' }) (or equivalent ids/defaults) in the TvDetails component where the h3 headings are rendered (ensure the component has access to the intl object via useIntl or injectIntl). Update the message definitions for those four keys and replace the literal strings with intl.formatMessage calls so the headings are translatable.src/components/TitleCard/index.tsx (2)
510-543:⚠️ Potential issue | 🟡 MinorDifferentiate
data-testidvalues between layout variants.Both the AMOLED title (line 510) and standard title (line 543) use
data-testid="title-card-title". This can break test queries that expect unique IDs.🧪 Suggested fix
-data-testid="title-card-title" // line 510 +data-testid="title-card-title-amoled" -data-testid="title-card-title" // line 543 +data-testid="title-card-title-standard"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TitleCard/index.tsx` around lines 510 - 543, Two h1 elements in the TitleCard component share the same data-testid ("title-card-title") causing conflicting test queries; update them to distinct values (for example change the AMOLED/first h1 to "title-card-title-amoled" and the standard/second h1 inside the title-card-standard-title-block to "title-card-title-standard") by locating the h1 with data-testid in the AMOLED title block and the h1 inside the element with className "title-card-standard-title-block", change their data-testid strings accordingly, and then update any tests that reference the old test id to use the new specific ids.
377-416:⚠️ Potential issue | 🟡 MinorAdd accessible labels to icon-only buttons.
The AMOLED icon buttons at lines 377-382, 389-394, and 406-411 lack accessible names. Screen reader users won't understand their purpose.
♿ Suggested fix
<button + type="button" + aria-label={intl.formatMessage(messages.addToWatchList)} className="title-card-amoled-icon-btn z-40 flex h-7 w-7 items-center justify-center rounded-full bg-black/50 backdrop-blur-md ring-1 ring-white/[0.12] transition hover:bg-black/70" onClick={onClickWatchlistBtn} >Apply the same pattern (
type="button"+aria-label) to the remove-watchlist button (line 389) and blocklist button (line 406).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TitleCard/index.tsx` around lines 377 - 416, The AMOLED icon-only <button> elements for removing from watchlist and for blocklist/hide lack accessible names; update the two plain <button> elements that call onClickDeleteWatchlistBtn and setShowBlocklistModal (the ones with className "title-card-amoled-icon-btn") to include type="button" and a descriptive aria-label (e.g., aria-label="Remove from watchlist" for onClickDeleteWatchlistBtn and aria-label="Hide title" or "Block title" for setShowBlocklistModal) so screen readers can announce their purpose; keep the existing visual content and handlers unchanged.
🧹 Nitpick comments (6)
src/components/Common/Tooltip/index.tsx (1)
28-28: Use semantic border utility to keep tooltip fully theme-aware.Line 28 still hardcodes
border-gray-600, which bypasses theme token remapping. Use a semantic border class so the tooltip border tracks AMOLED/default theme tokens.Proposed tweak
- `z-50 text-sm absolute font-normal px-2 py-1 rounded border shadow text-gray-100 bg-surface-raised border-gray-600`, + `z-50 text-sm absolute font-normal px-2 py-1 rounded border shadow text-gray-100 bg-surface-raised border-border-default`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/Tooltip/index.tsx` at line 28, The tooltip class string currently hardcodes "border-gray-600"; update the Tooltip component's className string (the template literal that includes "z-50 text-sm absolute font-normal px-2 py-1 rounded border shadow text-gray-100 bg-surface-raised border-gray-600") to use the semantic border utility (for example replace "border-gray-600" with a theme token class such as "border-surface-border") so the tooltip border follows theme token remapping.src/components/Common/SlideOver/index.tsx (1)
74-74: Consider migratingring-gray-700toring-ring-app/10for consistency.The
bg-surfacemigration is correct, butring-gray-700remains hardcoded while other components in this PR (e.g., OverrideRuleTiles, SettingsMetadata, Releases, UserLinkedAccountsSettings) use the semanticring-ring-app/10token.Suggested fix
- <div className={`flex h-full flex-col rounded-lg shadow-xl ring-1 backdrop-blur bg-surface ring-gray-700`}> + <div className="flex h-full flex-col rounded-lg shadow-xl ring-1 backdrop-blur bg-surface ring-ring-app/10">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/SlideOver/index.tsx` at line 74, The div in the SlideOver component currently hardcodes the tailwind class ring-gray-700; update the class list in the JSX element (the div with className starting "flex h-full flex-col rounded-lg shadow-xl ring-1 backdrop-blur bg-surface") to replace ring-gray-700 with the semantic token ring-ring-app/10 so it matches other components (e.g., OverrideRuleTiles, SettingsMetadata) and keeps styling consistent.src/components/Common/Modal/index.tsx (1)
119-119: Consider using the semantic ring token for consistency.The dialog uses
bg-surfacefor the background but retainsring-gray-700instead of using the semanticring-ring-apptoken. Other components in this PR (e.g., DeviceItem, RequestItem) usering-ring-app/10for consistency with the theme system.♻️ Suggested fix
- className={`hide-scrollbar relative inline-block w-full overflow-auto px-4 pb-4 pt-4 text-left align-bottom shadow-xl ring-1 transition-all sm:my-8 sm:max-w-3xl sm:rounded-lg sm:align-middle bg-surface ring-gray-700 ${dialogClass}`} + className={`hide-scrollbar relative inline-block w-full overflow-auto px-4 pb-4 pt-4 text-left align-bottom shadow-xl ring-1 transition-all sm:my-8 sm:max-w-3xl sm:rounded-lg sm:align-middle bg-surface ring-ring-app/10 ${dialogClass}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/Modal/index.tsx` at line 119, Replace the hardcoded ring color in the Modal dialog's container className (the template string in src/components/Common/Modal/index.tsx) with the semantic ring token used elsewhere (change ring-gray-700 to ring-ring-app/10) so the dialog uses the theme token consistently with components like DeviceItem and RequestItem; update the className string around the existing `dialogClass` usage to reference `ring-ring-app/10` instead of `ring-gray-700`.src/components/Discover/HeroSlider/index.tsx (2)
194-209: Inline styles for animations work but could be extracted.The inline
<style>block defines keyframe animations that are recreated on each render. While functional, consider moving these toglobals.cssor a CSS module for better caching and separation of concerns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/HeroSlider/index.tsx` around lines 194 - 209, The inline <style> block inside the HeroSlider component recreates keyframes on each render; move the keyframes (heroSlideIn, heroSlideOut) and the related classes (.hero-item, .hero-item-in, .hero-item-out) out of index.tsx into a shared stylesheet (e.g., globals.css or a CSS module) and import that stylesheet into the component, then remove the <style> block so the component uses the persisted CSS classes and animations instead of embedding them at render time.
68-80: Carousel interval runs even when component is hidden.Per Context snippet 2, HeroSlider is mounted inside a
hiddenCSS class div inDiscover/index.tsx. However, theuseEffectstill starts a 7-secondsetIntervaland the SWR fetch still runs, consuming resources for a component that's never displayed in the default theme.Consider either:
- Passing a prop to disable auto-rotation when hidden
- Using the theme context to conditionally enable the carousel
- Not mounting the component at all when not AMOLED
// Example: accept a disabled prop const HeroSlider = ({ disabled = false }: { disabled?: boolean }) => { // ... useEffect(() => { if (disabled || items.length <= 1) return; startInterval(); return () => { /* cleanup */ }; }, [items.length, startInterval, disabled]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/HeroSlider/index.tsx` around lines 68 - 80, HeroSlider currently always starts the rotation interval (startInterval) and triggers its fetch even when hidden; update the HeroSlider component to accept a prop (e.g., disabled?: boolean) or read the theme/context and gate both the interval and any SWR/data hooks: modify the useEffect that calls startInterval to early-return when disabled or theme indicates hidden (if disabled || items.length <= 1) and also avoid invoking the data-fetching hook (or return null) when disabled so no network/interval work runs when the component is hidden; reference startInterval, intervalRef, and the useEffect that currently depends on items.length/startInterval to implement these checks and cleanup.src/components/Discover/DiscoverTv/index.tsx (1)
68-75: Consider lazy-loading the backdrop based on theme.The AMOLED backdrop and associated useEffect run unconditionally on every page load, fetching images and maintaining state even when the default theme hides the element via CSS (
display: noneinglobals.css).For efficiency, consider gating this with the theme check, similar to
ImageFader(Context snippet 1):import useTheme from '@app/hooks/useTheme'; // ... const { theme } = useTheme(); const isAmoled = theme === 'amoled-strix'; useEffect(() => { if (!isAmoled || pickedRef.current || !titles?.length) return; // ... rest of backdrop selection logic }, [titles, isAmoled]); // In render: {isAmoled && backdropPath && ( <div className="discover-tv-amoled-backdrop ...">This avoids unnecessary image fetches and DOM nodes when the feature is hidden.
Also applies to: 86-102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/DiscoverTv/index.tsx` around lines 68 - 75, The backdrop selection useEffect (the effect using pickedRef, titles, setBackdropPath and backdropPath) should be gated by theme to avoid work when the AMOLED backdrop is hidden: import and call useTheme to get theme, derive isAmoled (theme === 'amoled-strix'), add isAmoled to the effect dependencies and short-circuit if !isAmoled, and only render the backdrop DOM (the element that uses backdropPath) when isAmoled && backdropPath; mirror the same change for the other effect/block referenced around lines 86-102 so both selection and rendering are lazy based on isAmoled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Layout/PullToRefresh/index.tsx`:
- Line 120: The PullToRefresh component's JSX contains hardcoded tailwind colors
in the class string (the fragment ending with "relative -top-28 h-9 w-9
rounded-full border-4 shadow-md shadow-black ring-1 border-gray-800
bg-surface-raised ring-gray-700") — replace the two hardcoded classes
`border-gray-800` and `ring-gray-700` with the project's semantic border/ring
tokens (for example use the semantic border token like `border-surface-border`
and a semantic ring token like `ring-surface-ring` or whatever token names your
design system exports) so the component uses theme tokens; update the className
in the PullToRefresh component accordingly and run a quick visual check in
dark/AMOLED themes.
In `@src/components/PersonDetails/index.tsx`:
- Around line 245-269: The two ImageFader instances in PersonDetails (inside the
person-fader-standard and person-fader-amoled divs) mount simultaneously causing
duplicate image loads and duplicate rotation intervals; fix by mounting a single
ImageFader: compute the backgroundImages once from sortedCast and sortedCrew,
render only one ImageFader (referencing ImageFader and the computed
backgroundImages array), and control the visual differences (the AMOLED gradient
overlay or darker styling) via CSS or a prop (e.g., isAmoled/isDarker) so only
one useEffect/interval runs—remove the duplicate ImageFader and move the extra
gradient div or conditional styling to overlay the single fader instead.
In `@src/pages/users/`[userId]/settings/appearance.tsx:
- Around line 5-11: Add the same route guard used by other user settings pages:
call useRouteGuard(Permission.MANAGE_USERS) at the top of the
UserAppearanceSettingsPage component (import useRouteGuard and Permission) so
only users with MANAGE_USERS can access /users/[userId]/settings/appearance;
alternatively, if you prefer the UX change, detect when router.query.userId !==
currentUser.id and redirect to /profile/settings/appearance (use router.push)
instead of rendering UserAppearanceSettings—update imports and ensure this logic
lives inside UserAppearanceSettingsPage before returning the JSX.
---
Outside diff comments:
In `@src/components/Settings/OverrideRule/OverrideRuleTiles.tsx`:
- Around line 155-163: The inner .map() iterations inside the OverrideRuleTiles
component render spans without key props (e.g., the users map over rule.users,
genres map, languages map, keywords map, and tags map), causing unstable
reconciliation; update each mapped element (the span or fragment returned in the
users.map, genres.map, languages.map, keywords.map, and tags.map callbacks) to
include a stable key prop derived from the item (use userId or the mapped
id/string such as Number(userId), genre id/name, language code/id, keyword
id/text, or tag id) so React can track them reliably (ensure keys are unique
within each list and use the parsed Number(userId) or the original identifier as
appropriate).
---
Duplicate comments:
In `@src/components/Common/Tag/index.tsx`:
- Line 11: The Tag component's root div (in src/components/Common/Tag/index.tsx
— the inline-flex cursor-pointer items-center rounded-full ... element) uses
fixed palette classes (ring-gray-600 and hover:bg-gray-700) which break theming;
replace those fixed classes with the project's semantic tokens (e.g., use the
theme's ring and hover background tokens such as ring-<semantic-token> and
hover:bg-<semantic-token> — e.g., ring-border/ ring-muted and
hover:bg-surface-hover or the equivalent tokens your design system defines) so
the ring and hover states adapt to AMOLED-Strix and other themes while keeping
the rest of the class list intact.
In `@src/components/Discover/HeroSlider/index.tsx`:
- Around line 239-244: The "View Details" text in the Link inside the HeroSlider
is hardcoded; add a message key (e.g., viewDetails) to the component's messages
definition (use defineMessages with namespace 'components.Discover.HeroSlider')
and replace the literal "View Details" with
intl.formatMessage(messages.viewDetails) in the Link (the element using
href={detailUrl}); this aligns with existing i18n usage in the component.
In `@src/components/MediaSlider/index.tsx`:
- Line 192: Replace the hardcoded "See all" string in the MediaSlider component
with a localized message: import and use your app's i18n helper (e.g.,
useTranslation from 'react-i18next' or the project's t() utility) inside the
MediaSlider component and replace the literal "See all" JSX text with
t('mediaSlider.seeAll') (or the project's chosen key); also add the
'mediaSlider.seeAll' entry to the locale resource files for supported languages
so translations are available.
In `@src/components/MovieDetails/index.tsx`:
- Line 1411: Add localized labels for the AMOLED headings by extending the
existing messages object (identifier: messages) with keys crew, keywords,
ratings, and details and then replace the hardcoded strings "Crew", "Keywords",
"Ratings", and "Details" in the JSX with intl.formatMessage(messages.crew),
intl.formatMessage(messages.keywords), intl.formatMessage(messages.ratings), and
intl.formatMessage(messages.details) respectively; update usages near the h3
headings (the elements rendering those uppercase labels) so they pull from
intl.formatMessage instead of literal strings.
- Around line 1136-1139: The JSX contains a stray ternary closing token ") : ("
in the MovieDetails component causing a syntax error; fix it by introducing a
proper conditional around the two layout branches (standard vs AMOLED) — e.g.,
get theme from useTheme() (const { theme } = useTheme()) and compute isAmoled
(theme === 'amoled-strix'), then replace the fragment-wrapped pair with a
ternary expression using isAmoled ? <div className="relative bg-black">…</div> :
<div className="movie-details-backdrop-section …">…</div> (or invert the order
to match intended UX) so the opening condition matches the existing ") : (" and
removes the stray token.
In `@src/components/TitleCard/index.tsx`:
- Around line 510-543: Two h1 elements in the TitleCard component share the same
data-testid ("title-card-title") causing conflicting test queries; update them
to distinct values (for example change the AMOLED/first h1 to
"title-card-title-amoled" and the standard/second h1 inside the
title-card-standard-title-block to "title-card-title-standard") by locating the
h1 with data-testid in the AMOLED title block and the h1 inside the element with
className "title-card-standard-title-block", change their data-testid strings
accordingly, and then update any tests that reference the old test id to use the
new specific ids.
- Around line 377-416: The AMOLED icon-only <button> elements for removing from
watchlist and for blocklist/hide lack accessible names; update the two plain
<button> elements that call onClickDeleteWatchlistBtn and setShowBlocklistModal
(the ones with className "title-card-amoled-icon-btn") to include type="button"
and a descriptive aria-label (e.g., aria-label="Remove from watchlist" for
onClickDeleteWatchlistBtn and aria-label="Hide title" or "Block title" for
setShowBlocklistModal) so screen readers can announce their purpose; keep the
existing visual content and handlers unchanged.
In `@src/components/TvDetails/index.tsx`:
- Around line 360-362: The JSX currently has a broken ternary in the TvDetails
component: replace the stray `) : (` branch switch with a proper conditional
using a boolean (e.g., isAmoled) so the component renders either the standard
layout or the AMOLED layout (turn the fragment split into isAmoled ? (standard
JSX) : (amoled JSX)); ensure the boolean isAmoled is defined/derived in
TvDetails (or passed as a prop) and use it in the ternary, and convert the bare
comment "AMOLED full-bleed layout" into a JSX comment wrapped in braces `{/*
AMOLED full-bleed layout */}` so the file parses correctly.
- Line 1670: The hardcoded section headings ("Crew", "Keywords", "Ratings",
"Details") in the TvDetails component should be replaced with localized
messages; add message identifiers (e.g.
tvDetails.crew/tvDetails.keywords/tvDetails.ratings/tvDetails.details) to the
messages file and use intl.formatMessage({ id: 'tvDetails.crew', defaultMessage:
'Crew' }) (or equivalent ids/defaults) in the TvDetails component where the h3
headings are rendered (ensure the component has access to the intl object via
useIntl or injectIntl). Update the message definitions for those four keys and
replace the literal strings with intl.formatMessage calls so the headings are
translatable.
In `@src/styles/globals.css`:
- Around line 657-663: The CSS rule for the selectors
.collection-backdrop-overlay and .issue-backdrop-overlay uses Tailwind's
theme('colors.gray.900') which won't be resolved in plain CSS; replace that call
with a concrete color value or a CSS variable (for example the equivalent
rgba/hex for gray.900 or var(--color-gray-900)) and ensure --tw-gradient-to is
set if you want theme-aware behavior so the background-image uses a valid color
instead of theme('colors.gray.900').
---
Nitpick comments:
In `@src/components/Common/Modal/index.tsx`:
- Line 119: Replace the hardcoded ring color in the Modal dialog's container
className (the template string in src/components/Common/Modal/index.tsx) with
the semantic ring token used elsewhere (change ring-gray-700 to
ring-ring-app/10) so the dialog uses the theme token consistently with
components like DeviceItem and RequestItem; update the className string around
the existing `dialogClass` usage to reference `ring-ring-app/10` instead of
`ring-gray-700`.
In `@src/components/Common/SlideOver/index.tsx`:
- Line 74: The div in the SlideOver component currently hardcodes the tailwind
class ring-gray-700; update the class list in the JSX element (the div with
className starting "flex h-full flex-col rounded-lg shadow-xl ring-1
backdrop-blur bg-surface") to replace ring-gray-700 with the semantic token
ring-ring-app/10 so it matches other components (e.g., OverrideRuleTiles,
SettingsMetadata) and keeps styling consistent.
In `@src/components/Common/Tooltip/index.tsx`:
- Line 28: The tooltip class string currently hardcodes "border-gray-600";
update the Tooltip component's className string (the template literal that
includes "z-50 text-sm absolute font-normal px-2 py-1 rounded border shadow
text-gray-100 bg-surface-raised border-gray-600") to use the semantic border
utility (for example replace "border-gray-600" with a theme token class such as
"border-surface-border") so the tooltip border follows theme token remapping.
In `@src/components/Discover/DiscoverTv/index.tsx`:
- Around line 68-75: The backdrop selection useEffect (the effect using
pickedRef, titles, setBackdropPath and backdropPath) should be gated by theme to
avoid work when the AMOLED backdrop is hidden: import and call useTheme to get
theme, derive isAmoled (theme === 'amoled-strix'), add isAmoled to the effect
dependencies and short-circuit if !isAmoled, and only render the backdrop DOM
(the element that uses backdropPath) when isAmoled && backdropPath; mirror the
same change for the other effect/block referenced around lines 86-102 so both
selection and rendering are lazy based on isAmoled.
In `@src/components/Discover/HeroSlider/index.tsx`:
- Around line 194-209: The inline <style> block inside the HeroSlider component
recreates keyframes on each render; move the keyframes (heroSlideIn,
heroSlideOut) and the related classes (.hero-item, .hero-item-in,
.hero-item-out) out of index.tsx into a shared stylesheet (e.g., globals.css or
a CSS module) and import that stylesheet into the component, then remove the
<style> block so the component uses the persisted CSS classes and animations
instead of embedding them at render time.
- Around line 68-80: HeroSlider currently always starts the rotation interval
(startInterval) and triggers its fetch even when hidden; update the HeroSlider
component to accept a prop (e.g., disabled?: boolean) or read the theme/context
and gate both the interval and any SWR/data hooks: modify the useEffect that
calls startInterval to early-return when disabled or theme indicates hidden (if
disabled || items.length <= 1) and also avoid invoking the data-fetching hook
(or return null) when disabled so no network/interval work runs when the
component is hidden; reference startInterval, intervalRef, and the useEffect
that currently depends on items.length/startInterval to implement these checks
and cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 780101fd-ffec-45a5-8d43-7cfb9182cb1a
📒 Files selected for processing (71)
.github/workflows/release.ymlsrc/components/Blocklist/index.tsxsrc/components/CollectionDetails/index.tsxsrc/components/Common/ButtonWithDropdown/index.tsxsrc/components/Common/Dropdown/index.tsxsrc/components/Common/ImageFader/index.tsxsrc/components/Common/Modal/index.tsxsrc/components/Common/PlayButton/index.tsxsrc/components/Common/SettingsTabs/index.tsxsrc/components/Common/SlideOver/index.tsxsrc/components/Common/Table/index.tsxsrc/components/Common/Tag/index.tsxsrc/components/Common/Tooltip/index.tsxsrc/components/Discover/DiscoverMovies/index.tsxsrc/components/Discover/DiscoverSliderEdit/index.tsxsrc/components/Discover/DiscoverTv/index.tsxsrc/components/Discover/HeroSlider/index.tsxsrc/components/Discover/Trending.tsxsrc/components/Discover/index.tsxsrc/components/IssueDetails/IssueComment/index.tsxsrc/components/IssueDetails/index.tsxsrc/components/IssueList/IssueItem/index.tsxsrc/components/IssueList/index.tsxsrc/components/IssueModal/CreateIssueModal/index.tsxsrc/components/Layout/AmoledNavbar/index.tsxsrc/components/Layout/PullToRefresh/index.tsxsrc/components/Layout/UserDropdown/index.tsxsrc/components/Layout/index.tsxsrc/components/Login/JellyfinLogin.tsxsrc/components/Login/LocalLogin.tsxsrc/components/Login/index.tsxsrc/components/MediaSlider/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/PersonDetails/index.tsxsrc/components/RegionSelector/index.tsxsrc/components/RequestBlock/index.tsxsrc/components/RequestButton/index.tsxsrc/components/RequestCard/index.tsxsrc/components/RequestList/RequestItem/index.tsxsrc/components/RequestList/index.tsxsrc/components/RequestModal/AdvancedRequester/index.tsxsrc/components/ResetPassword/RequestResetLink.tsxsrc/components/ResetPassword/index.tsxsrc/components/Settings/OverrideRule/OverrideRuleTiles.tsxsrc/components/Settings/SettingsAbout/Releases/index.tsxsrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsLogs/index.tsxsrc/components/Settings/SettingsMetadata.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/Settings/SettingsServices.tsxsrc/components/Setup/JellyfinSetup.tsxsrc/components/Setup/index.tsxsrc/components/Slider/index.tsxsrc/components/TitleCard/ErrorCard.tsxsrc/components/TitleCard/index.tsxsrc/components/Toast/index.tsxsrc/components/TvDetails/Season/index.tsxsrc/components/TvDetails/index.tsxsrc/components/UserList/index.tsxsrc/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsxsrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsWebPush/DeviceItem.tsxsrc/components/UserProfile/UserSettings/index.tsxsrc/components/UserProfile/index.tsxsrc/context/ThemeContext.tsxsrc/hooks/useTheme.tssrc/pages/_app.tsxsrc/pages/profile/settings/appearance.tsxsrc/pages/users/[userId]/settings/appearance.tsxsrc/styles/globals.csstailwind.config.js
✅ Files skipped from review due to trivial changes (1)
- src/components/Setup/JellyfinSetup.tsx
🚧 Files skipped from review as they are similar to previous changes (32)
- src/components/Settings/SettingsJellyfin.tsx
- src/components/Discover/index.tsx
- src/components/UserProfile/index.tsx
- src/components/Toast/index.tsx
- src/components/IssueList/index.tsx
- src/components/Settings/SettingsLogs/index.tsx
- src/components/RequestCard/index.tsx
- src/components/RequestBlock/index.tsx
- src/components/Common/Table/index.tsx
- src/components/RequestModal/AdvancedRequester/index.tsx
- src/components/RequestList/index.tsx
- src/components/Discover/DiscoverSliderEdit/index.tsx
- src/components/ResetPassword/RequestResetLink.tsx
- src/hooks/useTheme.ts
- src/pages/_app.tsx
- src/components/IssueDetails/IssueComment/index.tsx
- src/components/Blocklist/index.tsx
- src/components/Settings/SettingsServices.tsx
- src/components/Login/LocalLogin.tsx
- src/components/Login/index.tsx
- src/components/TitleCard/ErrorCard.tsx
- src/components/Layout/AmoledNavbar/index.tsx
- src/components/RegionSelector/index.tsx
- src/components/Layout/UserDropdown/index.tsx
- src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx
- src/components/UserList/index.tsx
- src/components/Slider/index.tsx
- src/components/Settings/SettingsPlex.tsx
- src/pages/profile/settings/appearance.tsx
- src/components/Discover/DiscoverMovies/index.tsx
- src/components/ResetPassword/index.tsx
- src/context/ThemeContext.tsx
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/styles/globals.css (1)
660-663:⚠️ Potential issue | 🟡 MinorFix Stylelint spacing violation in overlay gradient rule.
Line 662 triggers
declaration-empty-line-before; this can fail style checks.Suggested fix
[data-theme='amoled-strix'] .collection-backdrop-overlay, [data-theme='amoled-strix'] .issue-backdrop-overlay { --tw-gradient-from: rgba(0, 0, 0, 0.6); --tw-gradient-stops: var(--tw-gradient-from), var(--tw-gradient-to); + background-image: linear-gradient(to bottom, rgba(0, 0, 0, 0.6), rgb(17, 24, 39)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/styles/globals.css` around lines 660 - 663, The stylelint `declaration-empty-line-before` violation is triggered by the `--tw-gradient-stops` declaration; fix it by adding a single empty line immediately before the `--tw-gradient-stops` line in the same CSS block so the declaration has the required blank line separation (adjust the block containing `--tw-gradient-from`, `--tw-gradient-stops`, and `background-image`).src/components/TitleCard/index.tsx (1)
377-414:⚠️ Potential issue | 🟠 MajorAdd accessible names and explicit button type to icon-only action controls.
Line 377, Line 390, and Line 406 (
<button>) plus Line 383, Line 395, and Line 412 (<Button>) are icon-only and still have no accessible label; they should also usetype="button"explicitly.♿ Suggested fix
+// messages +removeFromWatchList: 'Remove from watchlist', +addToBlockList: 'Add to blocklist', -<button +<button + type="button" + aria-label={intl.formatMessage(messages.addToWatchList)} className="title-card-amoled-icon-btn ..." onClick={onClickWatchlistBtn} > -<Button buttonType={'ghost'} className="title-card-standard-icon-btn z-40" buttonSize={'sm'} onClick={onClickWatchlistBtn}> +<Button + buttonType="ghost" + aria-label={intl.formatMessage(messages.addToWatchList)} + className="title-card-standard-icon-btn z-40" + buttonSize="sm" + onClick={onClickWatchlistBtn} +> -<button +<button + type="button" + aria-label={intl.formatMessage(messages.removeFromWatchList)} className="title-card-amoled-icon-btn ..." onClick={onClickDeleteWatchlistBtn} > -<Button className="title-card-standard-icon-btn z-40" buttonSize={'sm'} onClick={onClickDeleteWatchlistBtn}> +<Button + buttonType="ghost" + aria-label={intl.formatMessage(messages.removeFromWatchList)} + className="title-card-standard-icon-btn z-40" + buttonSize="sm" + onClick={onClickDeleteWatchlistBtn} +> -<button +<button + type="button" + aria-label={intl.formatMessage(messages.addToBlockList)} className="title-card-amoled-icon-btn ..." onClick={() => setShowBlocklistModal(true)} > -<Button buttonType={'ghost'} className="title-card-standard-icon-btn z-40" buttonSize={'sm'} onClick={() => setShowBlocklistModal(true)}> +<Button + buttonType="ghost" + aria-label={intl.formatMessage(messages.addToBlockList)} + className="title-card-standard-icon-btn z-40" + buttonSize="sm" + onClick={() => setShowBlocklistModal(true)} +>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TitleCard/index.tsx` around lines 377 - 414, The icon-only native <button> elements and the custom <Button> instances in TitleCard (the ones rendering StarIcon, MinusCircleIcon, and EyeSlashIcon) need explicit type="button" and accessible labels; update each native <button> (e.g., the two StarIcon/MinusCircleIcon/EyeSlashIcon <button> tags) to include type="button" and aria-label attributes (e.g., aria-label="Add to watchlist", "Remove from watchlist", "Hide title"), and pass a type="button" prop plus an aria-label prop to the corresponding <Button> components (or wrap a visually-hidden text label) so screen readers can announce the action while keeping the existing onClick handlers (onClickWatchlistBtn, onClickDeleteWatchlistBtn, setShowBlocklistModal).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/TitleCard/index.tsx`:
- Around line 377-414: The icon-only native <button> elements and the custom
<Button> instances in TitleCard (the ones rendering StarIcon, MinusCircleIcon,
and EyeSlashIcon) need explicit type="button" and accessible labels; update each
native <button> (e.g., the two StarIcon/MinusCircleIcon/EyeSlashIcon <button>
tags) to include type="button" and aria-label attributes (e.g., aria-label="Add
to watchlist", "Remove from watchlist", "Hide title"), and pass a type="button"
prop plus an aria-label prop to the corresponding <Button> components (or wrap a
visually-hidden text label) so screen readers can announce the action while
keeping the existing onClick handlers (onClickWatchlistBtn,
onClickDeleteWatchlistBtn, setShowBlocklistModal).
In `@src/styles/globals.css`:
- Around line 660-663: The stylelint `declaration-empty-line-before` violation
is triggered by the `--tw-gradient-stops` declaration; fix it by adding a single
empty line immediately before the `--tw-gradient-stops` line in the same CSS
block so the declaration has the required blank line separation (adjust the
block containing `--tw-gradient-from`, `--tw-gradient-stops`, and
`background-image`).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ff508d56-648b-4b2a-95fb-c35a4a3a1c66
📒 Files selected for processing (9)
src/components/Common/Tag/index.tsxsrc/components/Discover/Trending.tsxsrc/components/IssueModal/CreateIssueModal/index.tsxsrc/components/Layout/PullToRefresh/index.tsxsrc/components/Login/JellyfinLogin.tsxsrc/components/Login/LocalLogin.tsxsrc/components/Settings/OverrideRule/OverrideRuleTiles.tsxsrc/components/TitleCard/index.tsxsrc/styles/globals.css
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Layout/PullToRefresh/index.tsx
- src/components/Login/LocalLogin.tsx
- src/components/IssueModal/CreateIssueModal/index.tsx
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Discover/HeroSlider/index.tsx`:
- Around line 74-79: The modal opens while the slider auto-rotates so
RequestModal reads mutable featured props and can request/apply status to the
wrong slide; fix by capturing and passing a frozen target into the modal when
opening. Specifically, when opening the RequestModal (the handler that currently
reads featured.tmdbId/type), create and store a frozen object (e.g.,
selectedRequestTarget with tmdbId and type) and pass that into RequestModal
instead of letting it read from featured; also set a flag (e.g.,
isRequestModalOpen) so startInterval/next/intervalRef/isPaused respect it (pause
rotation or prevent featured updates) while the modal is open. Apply the same
change to the other modal-opening handlers referenced (the blocks around lines
143-153 and 251-259) so all modal openings use the frozen selectedRequestTarget
rather than the live featured value.
- Around line 81-90: When items changes it can become shorter than the existing
currentIndex, so accessing items[currentIndex] may be undefined; update the
component to clamp or reset currentIndex whenever items.length changes (e.g., in
the existing useEffect or a new useEffect that depends on items.length) so
currentIndex = Math.min(currentIndex, Math.max(0, items.length - 1)) or set to 0
when items.length === 0, and also guard any subsequent accesses (e.g., where
`featured = items[currentIndex]` and the rendering block around that and the
code referenced at lines 95-101) by checking that featured is defined before
reading its properties or rendering its UI.
- Around line 156-159: The backdrop stack keys use only item.id which can
collide between movies and TV shows; update the key in the items.map render (in
the HeroSlider component) to use a composite key including the media type and id
(e.g., combine item.type or item.media_type with item.id, with a fallback to the
map index if type is missing) so each element has a stable, unique key across
mixed media.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7713591a-c43f-44d3-9f35-f2ecd8dc4f7e
📒 Files selected for processing (2)
src/components/Discover/HeroSlider/index.tsxsrc/components/MediaSlider/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/MediaSlider/index.tsx
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/components/MovieDetails/index.tsx (1)
1140-1143:⚠️ Potential issue | 🔴 CriticalCritical: Invalid JSX — broken ternary with missing condition.
The return block opens with a fragment
<>at line 438, but line 1141 contains) : (which is the else-branch of a ternary operator without a corresponding opening condition. This is a syntax error that will prevent the code from compiling.Additionally:
useThemehook is not imported- No condition variable (e.g.,
isAmoled) is defined to gate the layout branches- Line 1142's comment must use JSX syntax:
{/* comment */}(flagged by Biome)To fix, import the theme hook and add a proper conditional:
🐛 Proposed structural fix
import useSettings from '@app/hooks/useSettings'; +import useTheme from '@app/hooks/useTheme'; import { Permission, UserType, useUser } from '@app/hooks/useUser';Then at the start of the component:
const MovieDetails = ({ movie }: MovieDetailsProps) => { const settings = useSettings(); + const { theme } = useTheme(); + const isAmoled = theme === 'amoled-strix'; const { user, hasPermission } = useUser();Then restructure the return statement:
return ( - <> - <div - className="movie-details-backdrop-section media-page" + isAmoled ? ( + {/* ── AMOLED full-bleed layout ── */} + <div className="relative bg-black"> ... </div> - ) : ( - /* ── AMOLED full-bleed layout ── */ - <div className="relative bg-black"> + ) : ( + <div + className="movie-details-backdrop-section media-page" ... </div> - </> + ) );,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` around lines 1140 - 1143, The JSX currently has a broken ternary in the MovieDetails component (the " ) : ( " else branch has no opening condition) and is missing the theme hook and a condition variable; fix by importing useTheme and inside the MovieDetails component derive a boolean (e.g., isAmoled) from useTheme (or theme.mode) and use that boolean as the ternary condition that chooses between the two layouts, ensure the top-level fragment/return wraps the full expression so the ternary is complete, and replace the HTML comment on line 1142 with a JSX comment syntax {/* ... */} so the JSX compiles.src/components/TvDetails/index.tsx (1)
1364-1365:⚠️ Potential issue | 🔴 CriticalCritical: Broken ternary expression makes this component unparseable.
Line 1364 contains
) : (which is the else branch of a ternary operator, but there is no opening? (before it. The standard layout (lines 470-1363) is not wrapped in any conditional expression. This code will fail to compile.Additionally, the comment on line 1365 is not wrapped in braces, which Biome correctly flags.
To fix this, you need to:
- Add a theme conditional with
useThemehook and wrap the two layouts in a proper ternary:isAmoled ? (AMOLED layout) : (Standard layout)- OR remove the stray
) : (entirely if both layouts should be CSS-controlled via[data-theme]selectors🐛 Proposed fix using theme conditional
+import { useTheme } from '@app/context/ThemeContext'; // ... other imports const TvDetails = ({ tv }: TvDetailsProps) => { + const { theme } = useTheme(); + const isAmoled = theme === 'amoled-strix'; const settings = useSettings(); // ... rest of component return ( <> + {isAmoled ? ( + {/* ── AMOLED full-bleed layout ── */} + <div className="relative bg-black"> + {/* ... AMOLED content ... */} + </div> + ) : ( <div className="tv-details-backdrop-section media-page" /* ... */> {/* ... Standard layout content ... */} </div> - ) : ( - /* ── AMOLED full-bleed layout ── */ - <div className="relative bg-black"> - {/* ... */} - </div> + )} </> ); };Run the following script to verify the JSX structure and check if
useThemeis available:#!/bin/bash # Check if the component currently imports useTheme rg -n "useTheme" src/components/TvDetails/index.tsx # Check if ThemeContext exists and exports useTheme fd -t f "ThemeContext" --exec cat {} 2>/dev/null | head -50 # Check for any isAmoled variable in the component rg -n "isAmoled" src/components/TvDetails/index.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` around lines 1364 - 1365, The JSX contains a stray "):(" else-branch and an unbraced comment making the component unparsable; fix it by importing and using the theme hook (e.g., import { useTheme } from ThemeContext), derive a boolean (e.g., const isAmoled = useTheme().name === 'amoled' or similar) inside the TvDetails component, and wrap the two layout fragments in a proper ternary like isAmoled ? (/* AMOLED full-bleed layout */ ...) : (/* Standard layout */ ...); alternatively, if theme switching is handled by CSS, remove the stray ") : (" and ensure the comment is inside JSX braces or moved outside JSX so the file parses (references: TvDetails component, useTheme, isAmoled).
🧹 Nitpick comments (3)
src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx (1)
110-111: Localize theme name/description instead of rendering raw literals.Line 110 and Line 111 bypass
react-intl, so these strings remain English in localized UIs. Consider mapping theme IDs to localized messages (or storing message keys in theme definitions).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx` around lines 110 - 111, UserAppearanceSettings currently renders theme strings directly via t.name and t.description, bypassing react-intl; replace those raw literals with localized messages by mapping each theme ID (or add a message key to the theme definition) to an intl message descriptor and render via formatMessage or <FormattedMessage>. Locate usages of t.name and t.description in the UserAppearanceSettings component and swap them to look up the correct message descriptor (e.g., themeLabels[theme.id] or theme.messageId) and call intl.formatMessage(messageDescriptor) (or <FormattedMessage id={...} defaultMessage={...} />) so names and descriptions are localized. Ensure you add corresponding message IDs to your messages file and fall back to the original string if no message is found.src/components/Layout/AmoledNavbar/index.tsx (1)
100-108: Consider moving keyframe animation to globals.css.The inline
<style>tag is recreated on every render. While this works, moving the@keyframes iconPopdefinition toglobals.csswould be slightly more efficient and keep the component JSX cleaner.♻️ Proposed refactor
Add to
src/styles/globals.css:`@keyframes` iconPop { from { opacity: 0; transform: scale(0.6) rotate(-15deg); } to { opacity: 1; transform: scale(1) rotate(0deg); } } .icon-pop { animation: iconPop 250ms cubic-bezier(0.34, 1.56, 0.64, 1) both; }Then remove lines 100-108 from the component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Layout/AmoledNavbar/index.tsx` around lines 100 - 108, Move the inline keyframe and animation CSS out of the AmoledNavbar component: remove the <style> block that defines `@keyframes` iconPop and .icon-pop from src/components/Layout/AmoledNavbar/index.tsx and instead add those rules to your global stylesheet (e.g., src/styles/globals.css). Keep the .icon-pop class usage in the JSX unchanged so elements still use the animation, but ensure `@keyframes` iconPop and the .icon-pop rule live in globals.css to avoid recreating the style tag on every render.src/components/TvDetails/index.tsx (1)
1901-1962: Consider extracting shared season logic to reduce duplication.The season status computation logic (lines 1914-1958) is identical to the standard layout (lines 793-838). This duplication spans ~150 lines of complex request/season filtering that will need to be maintained in sync.
Consider extracting the shared logic into helper functions or a custom hook to improve maintainability.
♻️ Example extraction approach
// Could be extracted to a helper or hook function useSeasonStatus(data: TvDetailsType, seasonNumber: number, is4k: boolean) { const mediaSeasons = data.mediaInfo?.seasons ?? []; const requests = data.mediaInfo?.requests ?? []; const mSeason = mediaSeasons.find( (s) => s.seasonNumber === seasonNumber && s[is4k ? 'status4k' : 'status'] !== MediaStatus.UNKNOWN ); const request = requests .filter((r) => r.seasons.some((s) => s.seasonNumber === seasonNumber) && r.is4k === is4k ) .sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime() )[0]; return { mSeason, request }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` around lines 1901 - 1962, The season status/request computation duplicated in TvDetails (variables mSeason, mSeason4k, request, request4k and show4k logic) should be extracted into a reusable helper or hook (e.g. useSeasonStatus or getSeasonRequests) that accepts data (TvDetailsType), seasonNumber and is4k and returns the computed mSeason and request; replace the repeated blocks in both the expanded-season layout (current block) and the standard layout (the earlier block that sets mSeason/request) with calls to that helper, and keep the show4k permission check in the component but pass is4k when calling the helper for 4k vs non-4k results.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx`:
- Around line 47-54: The theme buttons currently remove the default focus
outline and only show a visual selected state; restore keyboard focus visibility
and expose selection to assistive tech by adding focus-visible styling and
semantic pressed state: update the button rendered in UserAppearanceSettings
(the element that calls setTheme(t.id)) to remove the bare focus-outline removal
and instead include a visible focus-visible class (e.g., ring or outline on
focus-visible and a clear focus transition) and add aria-pressed={isActive} (or
aria-selected if part of a listbox) so screen readers know which theme is
active; apply the same changes to the other theme button block (the one around
lines 88-101) so both keyboard focus and pressed semantics are present.
---
Duplicate comments:
In `@src/components/MovieDetails/index.tsx`:
- Around line 1140-1143: The JSX currently has a broken ternary in the
MovieDetails component (the " ) : ( " else branch has no opening condition) and
is missing the theme hook and a condition variable; fix by importing useTheme
and inside the MovieDetails component derive a boolean (e.g., isAmoled) from
useTheme (or theme.mode) and use that boolean as the ternary condition that
chooses between the two layouts, ensure the top-level fragment/return wraps the
full expression so the ternary is complete, and replace the HTML comment on line
1142 with a JSX comment syntax {/* ... */} so the JSX compiles.
In `@src/components/TvDetails/index.tsx`:
- Around line 1364-1365: The JSX contains a stray "):(" else-branch and an
unbraced comment making the component unparsable; fix it by importing and using
the theme hook (e.g., import { useTheme } from ThemeContext), derive a boolean
(e.g., const isAmoled = useTheme().name === 'amoled' or similar) inside the
TvDetails component, and wrap the two layout fragments in a proper ternary like
isAmoled ? (/* AMOLED full-bleed layout */ ...) : (/* Standard layout */ ...);
alternatively, if theme switching is handled by CSS, remove the stray ") : ("
and ensure the comment is inside JSX braces or moved outside JSX so the file
parses (references: TvDetails component, useTheme, isAmoled).
---
Nitpick comments:
In `@src/components/Layout/AmoledNavbar/index.tsx`:
- Around line 100-108: Move the inline keyframe and animation CSS out of the
AmoledNavbar component: remove the <style> block that defines `@keyframes` iconPop
and .icon-pop from src/components/Layout/AmoledNavbar/index.tsx and instead add
those rules to your global stylesheet (e.g., src/styles/globals.css). Keep the
.icon-pop class usage in the JSX unchanged so elements still use the animation,
but ensure `@keyframes` iconPop and the .icon-pop rule live in globals.css to
avoid recreating the style tag on every render.
In `@src/components/TvDetails/index.tsx`:
- Around line 1901-1962: The season status/request computation duplicated in
TvDetails (variables mSeason, mSeason4k, request, request4k and show4k logic)
should be extracted into a reusable helper or hook (e.g. useSeasonStatus or
getSeasonRequests) that accepts data (TvDetailsType), seasonNumber and is4k and
returns the computed mSeason and request; replace the repeated blocks in both
the expanded-season layout (current block) and the standard layout (the earlier
block that sets mSeason/request) with calls to that helper, and keep the show4k
permission check in the component but pass is4k when calling the helper for 4k
vs non-4k results.
In `@src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx`:
- Around line 110-111: UserAppearanceSettings currently renders theme strings
directly via t.name and t.description, bypassing react-intl; replace those raw
literals with localized messages by mapping each theme ID (or add a message key
to the theme definition) to an intl message descriptor and render via
formatMessage or <FormattedMessage>. Locate usages of t.name and t.description
in the UserAppearanceSettings component and swap them to look up the correct
message descriptor (e.g., themeLabels[theme.id] or theme.messageId) and call
intl.formatMessage(messageDescriptor) (or <FormattedMessage id={...}
defaultMessage={...} />) so names and descriptions are localized. Ensure you add
corresponding message IDs to your messages file and fall back to the original
string if no message is found.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a135842-0047-43bd-bf77-412cc2528fb1
📒 Files selected for processing (5)
src/components/Layout/AmoledNavbar/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/TvDetails/index.tsxsrc/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsxsrc/pages/users/[userId]/settings/appearance.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/users/[userId]/settings/appearance.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx (1)
47-55:⚠️ Potential issue | 🟠 MajorRestore keyboard focus visibility and selected-state semantics on theme buttons.
Line 50 removes the focus outline without a visible replacement, and the active state is only visual. Please add explicit
focus-visiblestyling andaria-pressed(and settype="button").Suggested patch
<button key={t.id} + type="button" onClick={() => setTheme(t.id)} - className={`relative flex flex-col overflow-hidden rounded-xl border-2 text-left transition duration-200 focus:outline-none ${ + aria-pressed={isActive} + className={`relative flex flex-col overflow-hidden rounded-xl border-2 text-left transition duration-200 focus:outline-none ${ isActive ? 'border-indigo-500 shadow-lg shadow-indigo-500/20' : 'border-gray-700 hover:border-gray-500' - }`} + } focus-visible:ring-2 focus-visible:ring-indigo-500 focus-visible:ring-offset-2 focus-visible:ring-offset-black`} style={{ background: bgColor }} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx` around lines 47 - 55, The theme buttons in UserAppearanceSettings removed the default focus outline and lack accessibility semantics; update the button rendered in the map (the element with key={t.id} and onClick={() => setTheme(t.id)}) to include type="button", set aria-pressed={isActive} to reflect selected state, and replace the removed focus outline with an explicit visible focus-visible style (e.g. add focus-visible:ring and focus-visible:ring-offset classes or similar accessible focus indicator alongside any existing shadow/highlight classes) so keyboard users see focus.src/components/MovieDetails/index.tsx (1)
437-440:⚠️ Potential issue | 🔴 CriticalFix the broken ternary operator structure — this code will not compile.
Same issue as TvDetails: the return block opens with a fragment
<>at line 438, but line 1141 contains) : (which is the "else" branch of a ternary without a matching opening condition. This is invalid JSX.🐛 Proposed structural fix
return ( - <> - <div - className="movie-details-backdrop-section media-page" + isAmoled ? ( + /* ── AMOLED full-bleed layout ── */ + <div className="relative bg-black"> + {/* ... AMOLED content ... */} + </div> + ) : ( + <div + className="movie-details-backdrop-section media-page"You'll need to:
- Import the theme hook (e.g.,
useThemefrom the new ThemeProvider)- Add the condition variable (e.g.,
const { theme } = useTheme(); const isAmoled = theme === 'amoled-strix')- Wrap each layout branch properly in the ternary
- Remove or restructure the fragment wrapper at line 1707
Also applies to: 1140-1143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` around lines 437 - 440, The JSX return in MovieDetails is broken because a fragment <> is left open while a ternary's "else" branch `) : (` appears later (invalid JSX); fix by importing and using the theme hook (e.g., useTheme) inside the MovieDetails component to declare the condition (const { theme } = useTheme(); const isAmoled = theme === 'amoled-strix'), then restructure the return so the ternary is complete and each branch is wrapped in its own top-level element (remove the stray fragment wrapper around the whole return and ensure the conditionally rendered branches are returned as `isAmoled ? (<AmoledLayout ... />) : (<DefaultLayout ... />)` or equivalent), and update any references to the previous fragment children to live inside those branch elements so the JSX compiles.src/components/TvDetails/index.tsx (1)
468-471:⚠️ Potential issue | 🔴 CriticalFix the broken ternary operator structure — this code will not compile.
The return block opens with a fragment
<>at line 469, but line 1364 contains) : (which is the "else" branch of a ternary operator without a matching opening condition (condition ? (). This is invalid JSX syntax.The code needs a conditional to switch between the standard and AMOLED layouts. Based on the PR description, this should likely use a theme check.
🐛 Proposed structural fix
return ( - <> - <div - className="tv-details-backdrop-section media-page" + isAmoled ? ( + /* ── AMOLED full-bleed layout ── */ + <div className="relative bg-black"> + {/* ... AMOLED content ... */} + </div> + ) : ( + <div + className="tv-details-backdrop-section media-page"You'll need to:
- Import the theme hook (e.g.,
useThemeor check[data-theme='amoled-strix'])- Add the condition variable (e.g.,
const isAmoled = theme === 'amoled-strix')- Wrap each layout branch properly in the ternary
- Ensure the fragment
<>...</>wrapping at line 2248 is removed or properly structuredAlso applies to: 1364-1366
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` around lines 468 - 471, The JSX return contains an orphaned ternary "):(" else branch — fix by importing your theme hook (e.g., useTheme or reading data-theme), create a boolean like const isAmoled = theme === 'amoled-strix', and rewrite the top-level return to use isAmoled ? ( /* AMOLED layout JSX branch */ ) : ( /* standard layout JSX branch */ ), removing the stray fragment wrappers so each branch is a valid JSX expression; ensure the fragment opened at the original return is either removed or encompassed correctly inside the chosen conditional and that any helper render functions (if present) are called inside their respective branch.src/components/Discover/HeroSlider/index.tsx (1)
157-161:⚠️ Potential issue | 🟡 Minor
currentStatusmay display on the wrong slide after modal completion.The
requestTargetcorrectly freezes the item being requested, butcurrentStatusis unconditionally set on completion. If the user starts a request, the slide auto-rotates to a different item, and then the request completes,currentStatuswill be displayed on the new featured item rather than the one that was actually requested.Consider verifying the target matches before updating the status:
onComplete={(s) => { + if ( + requestTarget?.id === featured.id && + requestTarget?.type === (isMovie ? 'movie' : 'tv') + ) { setCurrentStatus(s); + } setShowRequestModal(false); setRequestTarget(null); }}Alternatively, pause auto-rotation while the modal is open by setting
isPaused.current = truewhen opening and resetting it on close.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/HeroSlider/index.tsx` around lines 157 - 161, The onComplete handler unconditionally sets currentStatus which can appear on a different slide if the carousel rotated after the request started; update the onComplete handler in the component (the function handling onComplete where setCurrentStatus, setShowRequestModal, setRequestTarget are called) to first verify the requestTarget still matches the currently featured item (e.g., compare requestTarget.id or unique key to the active slide id) before calling setCurrentStatus, or alternatively implement pause-until-complete behavior by setting isPaused.current = true when opening the modal and restoring it on close so the slide cannot auto-rotate while a request is in progress.src/components/TitleCard/index.tsx (1)
385-387:⚠️ Potential issue | 🟠 MajorAdd accessible names to icon-only standard
Buttoncontrols.The AMOLED native buttons are labeled, but the standard icon-only
Buttonvariants are still unlabeled. In the standard theme, these controls may be announced as unnamed buttons.♿ Suggested patch
-<Button buttonType={'ghost'} className="title-card-standard-icon-btn z-40" buttonSize={'sm'} onClick={onClickWatchlistBtn}> +<Button + buttonType={'ghost'} + aria-label={intl.formatMessage(messages.addToWatchList)} + className="title-card-standard-icon-btn z-40" + buttonSize={'sm'} + onClick={onClickWatchlistBtn} +> <StarIcon className={'h-3 text-amber-300'} /> </Button> -<Button className="title-card-standard-icon-btn z-40" buttonSize={'sm'} onClick={onClickDeleteWatchlistBtn}> +<Button + aria-label={intl.formatMessage(messages.removeFromWatchList)} + className="title-card-standard-icon-btn z-40" + buttonSize={'sm'} + onClick={onClickDeleteWatchlistBtn} +> <MinusCircleIcon className={'h-3'} /> </Button> -<Button buttonType={'ghost'} className="title-card-standard-icon-btn z-40" buttonSize={'sm'} onClick={() => setShowBlocklistModal(true)}> +<Button + buttonType={'ghost'} + aria-label={intl.formatMessage(globalMessages.blocklist)} + className="title-card-standard-icon-btn z-40" + buttonSize={'sm'} + onClick={() => setShowBlocklistModal(true)} +> <EyeSlashIcon className={'h-3'} /> </Button>Use this to verify
Buttonforwardsaria-labelto a native element (expected: props spread onto<button>):#!/bin/bash set -euo pipefail echo "Locate Common Button implementation:" fd "index.tsx" src/components/Common/Button echo echo "Inspect Button props + render (look for ...props forwarding to <button>):" fd "index.tsx" src/components/Common/Button --exec sed -n '1,260p' echo echo "Current icon-only standard buttons in TitleCard:" rg -n -C2 "title-card-standard-icon-btn|aria-label" src/components/TitleCard/index.tsxAlso applies to: 398-400, 416-418
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TitleCard/index.tsx` around lines 385 - 387, The icon-only standard Button instances in TitleCard (e.g., the Button with className "title-card-standard-icon-btn" that calls onClickWatchlistBtn) lack accessible names; add a descriptive aria-label (e.g., "Add to watchlist" or "Remove from watchlist" as appropriate) to each icon-only Button usage in TitleCard (also update the other occurrences around the referenced blocks). Also verify Common Button (src/components/Common/Button index.tsx) forwards props such as aria-label to the native <button> (ensure props are spread onto the rendered <button>), so the aria-label reaches the DOM.src/styles/globals.css (1)
658-662:⚠️ Potential issue | 🟡 MinorFix the remaining Stylelint violation in this gradient block.
Line 662 is still failing
declaration-empty-line-before, so this file will not lint clean until there's a blank line beforebackground-image.🧹 Minimal fix
[data-theme='amoled-strix'] .collection-backdrop-overlay, [data-theme='amoled-strix'] .issue-backdrop-overlay { --tw-gradient-from: rgba(0, 0, 0, 0.6); --tw-gradient-stops: var(--tw-gradient-from), var(--tw-gradient-to); + background-image: linear-gradient(to bottom, rgba(0, 0, 0, 0.6), rgb(17, 24, 39)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/styles/globals.css` around lines 658 - 662, The CSS block for [data-theme='amoled-strix'] .collection-backdrop-overlay and .issue-backdrop-overlay is missing a blank line before the background-image declaration which triggers the stylelint rule declaration-empty-line-before; fix it by adding a single blank line immediately above the background-image property inside that rule so the declarations are separated (i.e., ensure there's an empty line between the --tw-gradient-stops declaration and the background-image declaration).
🧹 Nitpick comments (3)
src/components/Discover/HeroSlider/index.tsx (2)
214-229: Inline<style>tag is recreated on every render.The
@keyframesdefinitions inside the JSX are re-inserted into the DOM on each render, which is inefficient. Consider moving these animations toglobals.cssor a CSS module.♻️ Move to globals.css
In
src/styles/globals.css, add:`@keyframes` heroSlideIn { from { opacity: 0; transform: translateX(22px); } to { opacity: 1; transform: translateX(0); } } `@keyframes` heroSlideOut { from { opacity: 1; transform: translateX(0); } to { opacity: 0; transform: translateX(-14px); } } .hero-item { animation-fill-mode: both; animation-timing-function: cubic-bezier(0.22, 1, 0.36, 1); } .hero-item-in { animation-name: heroSlideIn; animation-duration: 500ms; } .hero-item-out { animation-name: heroSlideOut; animation-duration: 250ms; }Then remove the
<style>tag from lines 214-229.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/HeroSlider/index.tsx` around lines 214 - 229, The inline <style> block that defines the hero animations (keyframes heroSlideIn and heroSlideOut and classes .hero-item, .hero-item-in, .hero-item-out) is being recreated on every render; move these definitions into a persistent stylesheet (e.g., add the `@keyframes` and the .hero-item / .hero-item-in / .hero-item-out rules into src/styles/globals.css or a component CSS module) and then remove the <style> tag from the HeroSlider JSX so the component references the class names but no longer injects styles on each render.
78-90: Auto-rotation runs even when component is hidden.The component is rendered inside
<div className="discover-hero-slider hidden">insrc/components/Discover/index.tsx. By default (non-amoled-strix theme), thehiddenclass appliesdisplay: none, but the component still:
- Fetches trending data via SWR on mount
- Runs the 7-second rotation interval continuously
- Preloads 8 backdrop images
Consider pausing the interval when the component is not visible:
- Pass a visibility prop and conditionally start the interval, or
- Use
IntersectionObserverto detect visibility, or- Check
document.hiddenin the interval callbackThis is a minor optimization for resource-conscious environments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/HeroSlider/index.tsx` around lines 78 - 90, The auto-rotation still runs when the slider is not visible; update the start/stop logic so the interval managed by startInterval / intervalRef and the useEffect that depends on items.length only starts when the component is visible: detect visibility (choose one approach) and gate startInterval/clearInterval on it — e.g., add a visible prop or create an IntersectionObserver in the component that sets a local isVisible ref/state, then change the useEffect to call startInterval only when isVisible && items.length > 1 and ensure the cleanup clears intervalRef when !isVisible; also ensure the interval callback checks document.hidden or isVisible (alongside isPaused.current) to avoid rotating while hidden.src/styles/globals.css (1)
670-679: Avoid CSS-only switching for duplicated media subtrees.These selectors keep both theme variants mounted and just flip
display.src/components/Layout/index.tsx:91-95already does this for the two navbars, and the same pattern here now applies to headers, backdrops, and TitleCard variants. On media-heavy screens that adds extra React work and can cause unnecessary image/layout work for the hidden branch. Prefer gating duplicated subtrees in the component layer with the active theme and reserve CSS selectors for purely visual deltas.Also applies to: 684-837
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/styles/globals.css` around lines 670 - 679, The CSS is currently toggling visibility by setting display for .layout-amoled-navbar / .layout-standard-navbar and similar selectors which keeps both DOM subtrees mounted and duplicates work; instead, gate those duplicated subtrees in the component layer (same approach used in src/components/Layout/index.tsx for the two navbars) so only the active variant is rendered. Update the Layout component (or the components that render headers, backdrops, and TitleCard variants) to switch on the active theme and render only the Amoled variant or the Standard variant, then remove or simplify the corresponding CSS display toggles (e.g., rules targeting .layout-amoled-navbar, .layout-standard-navbar and the duplicated header/backdrop/TitleCard selectors) so CSS only contains visual deltas rather than mounting logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/TitleCard/index.tsx`:
- Around line 345-355: The AMOLED letter badge is decorative and should be
hidden from assistive tech; update the TitleCard badge element (the div with
class "title-card-amoled-badge" or the inner span that renders 'M/C/S') to
include an accessibility attribute such as aria-hidden="true" (or
role="presentation") so screen readers ignore it and ensure it remains
non-focusable.
In `@src/styles/globals.css`:
- Around line 7-8: The app flashes because ThemeContext sets data-theme in a
useEffect (ThemeContext -> useEffect) after hydration; modify
src/pages/_document.tsx to add a small blocking inline script in the <Html> that
runs before React mounts to read the persisted theme (localStorage key used by
ThemeContext) and set document.documentElement.setAttribute('data-theme', value)
(falling back to default on missing/invalid values), or alternatively thread the
preference through server-side props so the initial HTML already has data-theme
on the <Html> element; ensure the script uses the exact storage key your
ThemeContext expects and handles invalid values safely so the pre-seeded
attribute matches what ThemeContext will apply.
---
Duplicate comments:
In `@src/components/Discover/HeroSlider/index.tsx`:
- Around line 157-161: The onComplete handler unconditionally sets currentStatus
which can appear on a different slide if the carousel rotated after the request
started; update the onComplete handler in the component (the function handling
onComplete where setCurrentStatus, setShowRequestModal, setRequestTarget are
called) to first verify the requestTarget still matches the currently featured
item (e.g., compare requestTarget.id or unique key to the active slide id)
before calling setCurrentStatus, or alternatively implement pause-until-complete
behavior by setting isPaused.current = true when opening the modal and restoring
it on close so the slide cannot auto-rotate while a request is in progress.
In `@src/components/MovieDetails/index.tsx`:
- Around line 437-440: The JSX return in MovieDetails is broken because a
fragment <> is left open while a ternary's "else" branch `) : (` appears later
(invalid JSX); fix by importing and using the theme hook (e.g., useTheme) inside
the MovieDetails component to declare the condition (const { theme } =
useTheme(); const isAmoled = theme === 'amoled-strix'), then restructure the
return so the ternary is complete and each branch is wrapped in its own
top-level element (remove the stray fragment wrapper around the whole return and
ensure the conditionally rendered branches are returned as `isAmoled ?
(<AmoledLayout ... />) : (<DefaultLayout ... />)` or equivalent), and update any
references to the previous fragment children to live inside those branch
elements so the JSX compiles.
In `@src/components/TitleCard/index.tsx`:
- Around line 385-387: The icon-only standard Button instances in TitleCard
(e.g., the Button with className "title-card-standard-icon-btn" that calls
onClickWatchlistBtn) lack accessible names; add a descriptive aria-label (e.g.,
"Add to watchlist" or "Remove from watchlist" as appropriate) to each icon-only
Button usage in TitleCard (also update the other occurrences around the
referenced blocks). Also verify Common Button (src/components/Common/Button
index.tsx) forwards props such as aria-label to the native <button> (ensure
props are spread onto the rendered <button>), so the aria-label reaches the DOM.
In `@src/components/TvDetails/index.tsx`:
- Around line 468-471: The JSX return contains an orphaned ternary "):(" else
branch — fix by importing your theme hook (e.g., useTheme or reading
data-theme), create a boolean like const isAmoled = theme === 'amoled-strix',
and rewrite the top-level return to use isAmoled ? ( /* AMOLED layout JSX branch
*/ ) : ( /* standard layout JSX branch */ ), removing the stray fragment
wrappers so each branch is a valid JSX expression; ensure the fragment opened at
the original return is either removed or encompassed correctly inside the chosen
conditional and that any helper render functions (if present) are called inside
their respective branch.
In `@src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx`:
- Around line 47-55: The theme buttons in UserAppearanceSettings removed the
default focus outline and lack accessibility semantics; update the button
rendered in the map (the element with key={t.id} and onClick={() =>
setTheme(t.id)}) to include type="button", set aria-pressed={isActive} to
reflect selected state, and replace the removed focus outline with an explicit
visible focus-visible style (e.g. add focus-visible:ring and
focus-visible:ring-offset classes or similar accessible focus indicator
alongside any existing shadow/highlight classes) so keyboard users see focus.
In `@src/styles/globals.css`:
- Around line 658-662: The CSS block for [data-theme='amoled-strix']
.collection-backdrop-overlay and .issue-backdrop-overlay is missing a blank line
before the background-image declaration which triggers the stylelint rule
declaration-empty-line-before; fix it by adding a single blank line immediately
above the background-image property inside that rule so the declarations are
separated (i.e., ensure there's an empty line between the --tw-gradient-stops
declaration and the background-image declaration).
---
Nitpick comments:
In `@src/components/Discover/HeroSlider/index.tsx`:
- Around line 214-229: The inline <style> block that defines the hero animations
(keyframes heroSlideIn and heroSlideOut and classes .hero-item, .hero-item-in,
.hero-item-out) is being recreated on every render; move these definitions into
a persistent stylesheet (e.g., add the `@keyframes` and the .hero-item /
.hero-item-in / .hero-item-out rules into src/styles/globals.css or a component
CSS module) and then remove the <style> tag from the HeroSlider JSX so the
component references the class names but no longer injects styles on each
render.
- Around line 78-90: The auto-rotation still runs when the slider is not
visible; update the start/stop logic so the interval managed by startInterval /
intervalRef and the useEffect that depends on items.length only starts when the
component is visible: detect visibility (choose one approach) and gate
startInterval/clearInterval on it — e.g., add a visible prop or create an
IntersectionObserver in the component that sets a local isVisible ref/state,
then change the useEffect to call startInterval only when isVisible &&
items.length > 1 and ensure the cleanup clears intervalRef when !isVisible; also
ensure the interval callback checks document.hidden or isVisible (alongside
isPaused.current) to avoid rotating while hidden.
In `@src/styles/globals.css`:
- Around line 670-679: The CSS is currently toggling visibility by setting
display for .layout-amoled-navbar / .layout-standard-navbar and similar
selectors which keeps both DOM subtrees mounted and duplicates work; instead,
gate those duplicated subtrees in the component layer (same approach used in
src/components/Layout/index.tsx for the two navbars) so only the active variant
is rendered. Update the Layout component (or the components that render headers,
backdrops, and TitleCard variants) to switch on the active theme and render only
the Amoled variant or the Standard variant, then remove or simplify the
corresponding CSS display toggles (e.g., rules targeting .layout-amoled-navbar,
.layout-standard-navbar and the duplicated header/backdrop/TitleCard selectors)
so CSS only contains visual deltas rather than mounting logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 608a5375-fd03-46a1-b1ea-88559010b7bf
📒 Files selected for processing (9)
src/components/Discover/HeroSlider/index.tsxsrc/components/Layout/AmoledNavbar/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/Slider/index.tsxsrc/components/TitleCard/index.tsxsrc/components/TvDetails/index.tsxsrc/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsxsrc/pages/users/[userId]/settings/appearance.tsxsrc/styles/globals.css
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Slider/index.tsx
- src/components/Layout/AmoledNavbar/index.tsx
- src/pages/users/[userId]/settings/appearance.tsx
Introduces a second UI theme - AMOLED-Strix - alongside the existing default dark theme. Designed for OLED/AMOLED displays with pure black backgrounds and violet accents to maximize power savings. ## What's included ### Theme system - New `ThemeContext` + `useTheme` hook with localStorage persistence - Appearance settings page at `/profile/settings` and `/users/[userId]/settings` - 7 semantic CSS color tokens (bg, surface, surface-raised, input, border, border-muted, ring) mapped to Tailwind utilities (bg-app, bg-surface, etc.) - All gray-* and indigo-* Tailwind classes route through CSS variables so both themes update automatically ### AMOLED-Strix visual design - Pure black (#000) background, near-black surfaces (#0e0e0e / #181818) - Violet accent replacing indigo on TV content; indigo retained for movies - Cinematic full-bleed hero on MovieDetails and TvDetails with backdrop blur, dark vignette, and staggered slide-in animations - Redesigned TitleCard: glass badge, stronger gradient, glass action buttons - Redesigned AmoledNavbar: floating glass search pill + profile dropdown - HeroSlider on Discover page: crossfade backdrop, dot indicators, swipe support - Redesigned MediaSlider section headers with accent divider bar - AMOLED-specific flat content sections with dividers instead of glass cards ### Code quality - Semantic token migration: removed all per-component `isAmoled` JS checks; structural differences handled via CSS `[data-theme='amoled-strix']` selectors - Accessible names on all icon-only buttons (slider nav, title card actions, hero slider dots) - i18n: all new UI strings go through intl.formatMessage (Crew, Keywords, Ratings, Details, See All, View Details, Go to slide N) - Missing React keys added throughout - Frozen request target in HeroSlider prevents wrong-item request on rotation - currentIndex guard when SWR returns fewer trending items - Composite mediaType+id key for HeroSlider backdrop stack - Route guard (MANAGE_USERS) on /users/[userId]/settings/appearance - Logout error handling in AmoledNavbar - Defensive swatch defaults in UserAppearanceSettings
RAFSuNX
force-pushed
the
feat/theming-system
branch
from
cf03b1b to
c5f5761
Compare
- Add aria-hidden to AMOLED badge div in TitleCard - Add type=button and aria-pressed to theme picker buttons in UserAppearanceSettings - Add missing deps (clear, setIsOpen) to AmoledNavbar useEffect
- Pause hero slider auto-rotation while request modal is open; resume on complete/cancel - Add focus-visible ring to theme picker buttons in UserAppearanceSettings - Add blank line before background-image in globals.css (stylelint) - Move theme name/description out of ThemeContext into i18n messages - Resolve localStorage synchronously before first render to eliminate theme flash - Add pre-hydration inline script to _document.tsx as fallback flash prevention
Mount one ImageFader instead of two hidden via CSS display toggle. AMOLED gradient overlay now controlled by a single CSS class (person-fader-amoled-overlay) so only one interval and image set runs.
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
src/components/TvDetails/index.tsx (1)
1364-1366:⚠️ Potential issue | 🔴 CriticalCritical: Broken ternary syntax — missing conditional operator.
Line 1364 contains
) : (but there is no matching ternary?operator that opens this conditional branch. The code appears to intend switching between standard and AMOLED layouts, but the ternary expression is incomplete and will cause a syntax/parse error preventing compilation.Additionally, line 1365's comment needs to be wrapped in JSX braces to comply with linting rules.
The fragment needs a proper ternary condition. Based on the PR's theme system, it should likely check the current theme:
🐛 Required fix
return ( - <> - <div + isAmoled ? ( + /* ── AMOLED full-bleed layout ── */ + <div className="relative bg-black"> + {/* ... AMOLED layout ... */} + </div> + ) : ( + <div className="tv-details-backdrop-section media-page" ...Or wrap the entire return in a proper conditional:
return isAmoled ? ( // AMOLED layout JSX ) : ( // Standard layout JSX );Note: You'll need to import and use
useThemeto get theisAmoledvalue:import useTheme from '@app/hooks/useTheme'; // ... const { theme } = useTheme(); const isAmoled = theme === 'amoled-strix';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` around lines 1364 - 1366, The JSX contains a broken ternary ") : (" without a matching "?" and an unwrapped JSX comment; fix by importing and using useTheme (e.g., call useTheme() and derive isAmoled via const isAmoled = theme === 'amoled-strix') and then wrap the return in a proper ternary like return isAmoled ? (/* AMOLED JSX */) : (/* standard JSX */); also convert the inline comment on the AMOLED branch to a JSX comment using {/* ... */} so linting passes; update references in this component (TvDetails/index.tsx) to use the new isAmoled condition.
🧹 Nitpick comments (11)
src/components/Common/SlideOver/index.tsx (1)
74-74: Use semantic ring token for full theme consistency.This container now uses
bg-surface, butring-gray-700is still hard-coded. Switching to the semantic ring utility keeps SlideOver fully theme-driven across Default/AMOLED-Strix.♻️ Suggested change
-<div className={`flex h-full flex-col rounded-lg shadow-xl ring-1 backdrop-blur bg-surface ring-gray-700`}> +<div className="flex h-full flex-col rounded-lg bg-surface shadow-xl ring-1 ring-ring-app backdrop-blur">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/SlideOver/index.tsx` at line 74, The SlideOver container div in the SlideOver component currently hard-codes the ring color (ring-gray-700); replace that class with the semantic ring token used across the design system (e.g., ring-outline) so the component follows theme tokens instead of a fixed color — update the class string in the div inside the SlideOver component (the line with `flex h-full flex-col rounded-lg shadow-xl ring-1 backdrop-blur bg-surface ring-gray-700`) to use the semantic ring class (replace `ring-gray-700` with your project’s semantic ring utility).src/components/Toast/index.tsx (1)
30-30: Use semantic ring token for full theme consistency.On Line 30,
bg-surfaceis theme-aware, butring-gray-500is still hardcoded. Switching to the semantic ring token keeps this toast aligned with the new token system across Default and AMOLED-Strix themes.Proposed diff
- <div className={`pointer-events-auto w-full max-w-sm rounded-lg shadow-lg ring-1 bg-surface ring-gray-500`}> + <div className="pointer-events-auto w-full max-w-sm rounded-lg bg-surface shadow-lg ring-1 ring-ring-app">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Toast/index.tsx` at line 30, Replace the hardcoded tailwind class "ring-gray-500" with the semantic ring token so the Toast stays theme-aware; update the class on the div in src/components/Toast/index.tsx (the element with className starting "pointer-events-auto w-full max-w-sm...") to use the semantic ring token (e.g., ring-ring) instead of ring-gray-500.src/components/Common/Table/index.tsx (1)
9-9: Use plain string literals for static class strings.Line 9 and Line 19 use template literals without interpolation. A plain string is simpler and avoids unnecessary syntax noise.
♻️ Suggested cleanup
- <tbody className={`divide-y divide-border-default bg-surface`}>{children}</tbody> + <tbody className="divide-y divide-border-default bg-surface">{children}</tbody>- `px-4 py-3 bg-surface-raised text-left text-xs leading-4 font-medium text-gray-200 uppercase tracking-wider truncate`, + 'px-4 py-3 bg-surface-raised text-left text-xs leading-4 font-medium text-gray-200 uppercase tracking-wider truncate',Also applies to: 19-19
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Common/Table/index.tsx` at line 9, Replace the unnecessary template literals used for static className values on the Table component's tbody elements with plain string literals: change className={`divide-y divide-border-default bg-surface`} (and the similar occurrence at line 19) to use regular quoted strings so the JSX uses "divide-y divide-border-default bg-surface" (and the corresponding static class string) instead of backtick template syntax.src/components/TitleCard/index.tsx (1)
488-490: Consider moving gradient to CSS custom property.The inline gradient uses hardcoded
rgba(45, 55, 72, ...)values. Per the PR's semantic token system, theme-aware colors should use CSS custom properties. Since this is wrapped intitle-card-detail-link, the gradient could be defined inglobals.cssunder[data-theme='amoled-strix']for theme-specific overrides.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TitleCard/index.tsx` around lines 488 - 490, The inline background gradient in TitleCard (component TitleCard, JSX element with class/title-card-detail-link) uses hardcoded rgba values; move that gradient into a CSS custom property (e.g. --title-card-detail-gradient) defined in globals.css and provide theme-specific overrides under [data-theme='amoled-strix'] so the semantic token system drives colors, then replace the inline style in TitleCard with a reference to that CSS variable. Ensure the CSS custom property is set to the same gradient string and that the JSX removes hardcoded rgba values and uses the property instead.src/components/MovieDetails/index.tsx (1)
1145-1171: Consider hoisting shared modals outside the conditional branches.The
IssueModal,ManageSlideOver, andBlocklistModalcomponents are identically duplicated in both the standard layout (lines 459-485) and AMOLED layout (lines 1145-1171). Since these modals are controlled by the same state variables and have identical props, they could be rendered once outside the conditional to reduce duplication and maintenance burden.♻️ Suggested structure
return ( <> + {/* Shared modals - rendered once regardless of theme */} + <PageTitle title={data.title} /> + <IssueModal ... /> + <ManageSlideOver ... /> + <BlocklistModal ... /> + {isAmoledTheme ? ( <div className="relative bg-black"> - <PageTitle title={data.title} /> - <IssueModal ... /> - <ManageSlideOver ... /> - <BlocklistModal ... /> {/* Hero and content sections */} ... </div> ) : ( <div className="movie-details-backdrop-section media-page" ...> - <PageTitle title={data.title} /> - <IssueModal ... /> - <ManageSlideOver ... /> - <BlocklistModal ... /> {/* Standard layout content */} ... </div> )} </> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` around lines 1145 - 1171, Duplicate modal components (IssueModal, ManageSlideOver, BlocklistModal) are rendered in both layout branches; hoist them out of the conditional so they render once and use the same state/handlers. Remove the duplicated instances inside the standard and AMOLED branches and place a single set of IssueModal (props: show={showIssueModal}, onCancel={() => setShowIssueModal(false)}, mediaType="movie", tmdbId={data.id}), ManageSlideOver (props: data={data}, mediaType="movie", show={showManager}, onClose={() => { setShowManager(false); router.push({ pathname: router.pathname, query: { movieId: router.query.movieId } }); }}, revalidate={() => revalidate()}), and BlocklistModal (props: tmdbId={data.id}, type="movie", show={showBlocklistModal}, onCancel={closeBlocklistModal}, onComplete={onClickHideItemBtn}, isUpdating={isBlocklistUpdating}) immediately outside the conditional that chooses standard vs AMOLED layout so all branches share these components and their state.src/components/UserProfile/index.tsx (1)
156-156: Minor inconsistency:ring-gray-700mixed with semanticbg-surface.The first quota card uses
ring-gray-700(hardcoded) while the other two conditionally usering-gray-700as well. For full theme consistency, consider usingring-ring-app/20orring-border-defaultto ensure proper appearance in both default and AMOLED themes.♻️ Suggested fix for consistency
- <div className={`overflow-hidden px-4 py-5 sm:p-6 rounded-lg bg-surface shadow ring-1 ring-gray-700`}> + <div className={`overflow-hidden px-4 py-5 sm:p-6 rounded-lg bg-surface shadow ring-1 ring-border-default`}>Similarly for lines 176 and 227, the non-restricted fallback case:
-${quota.movie.restricted ? 'bg-gradient-to-t from-red-900 to-transparent ring-red-500' : 'ring-gray-700'} +${quota.movie.restricted ? 'bg-gradient-to-t from-red-900 to-transparent ring-red-500' : 'ring-border-default'}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserProfile/index.tsx` at line 156, Replace the hardcoded ring class used on the quota card div in the UserProfile component with the semantic ring token used elsewhere so theme appearance stays consistent; specifically, update the div that currently uses `ring-gray-700` (and the equivalent non-restricted fallback cases referenced near the other quota card blocks) to use the semantic token such as `ring-ring-app/20` or `ring-border-default` so all quota card containers (the div with classes including `overflow-hidden px-4 py-5 sm:p-6 rounded-lg bg-surface shadow ring-1`) use the same theme-aware ring class.src/context/ThemeContext.tsx (1)
55-59: Consider guardingsetThemeagainst SSR edge cases.While
setThemeis typically only called from client-side interactions, adding a guard would make the code more defensive against potential SSR edge cases (e.g., if a component accidentally calls it during server rendering).♻️ Optional defensive guard
const setTheme = useCallback((next: ThemeId) => { setThemeState(next); + if (typeof window === 'undefined') return; localStorage.setItem(STORAGE_KEY, next); document.documentElement.setAttribute('data-theme', next); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/context/ThemeContext.tsx` around lines 55 - 59, The setTheme callback should be defensive for SSR: keep calling setThemeState(next) but wrap the client-only operations (localStorage.setItem and document.documentElement.setAttribute) in a runtime guard (e.g., check typeof window !== 'undefined' or typeof document !== 'undefined' and typeof localStorage !== 'undefined') so setTheme (the useCallback that calls setThemeState, localStorage.setItem, and document.documentElement.setAttribute) won't throw during server rendering; update the setTheme body to perform the guard before accessing localStorage or document.src/components/Setup/JellyfinSetup.tsx (1)
175-177: Theme token migration looks good.The hostname prefix span now uses semantic tokens (
border-border-default,bg-input) instead of hardcodedborder-gray-500/bg-gray-800, enabling proper theme adaptation.Minor: The template literal is unnecessary since there are no interpolated values.
♻️ Simplify to plain string
-<span className={`inline-flex cursor-default items-center rounded-l-md border border-r-0 px-3 text-gray-100 sm:text-sm border-border-default bg-input`}> +<span className="inline-flex cursor-default items-center rounded-l-md border border-r-0 px-3 text-gray-100 sm:text-sm border-border-default bg-input">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Setup/JellyfinSetup.tsx` around lines 175 - 177, The span's className currently uses a template literal without interpolation; change the className in the JSX span (the hostname prefix span that renders {values.useSsl ? 'https://' : 'http://'}) from a template literal to a plain string literal containing the classes (keep border-border-default, bg-input, inline-flex, cursor-default, items-center, rounded-l-md, border, border-r-0, px-3, text-gray-100, sm:text-sm) to simplify the code.src/components/Discover/HeroSlider/index.tsx (1)
216-231: Inline<style>tag creates duplicate styles on every render.The
<style>element inside the component body will be inserted into the DOM on every render, potentially creating duplicate style definitions. Consider moving these keyframe animations toglobals.cssor using a CSS-in-JS solution that handles deduplication.♻️ Move animations to globals.css
Add to
src/styles/globals.css:`@keyframes` heroSlideIn { from { opacity: 0; transform: translateX(22px); } to { opacity: 1; transform: translateX(0); } } `@keyframes` heroSlideOut { from { opacity: 1; transform: translateX(0); } to { opacity: 0; transform: translateX(-14px); } } .hero-item { animation-fill-mode: both; animation-timing-function: cubic-bezier(0.22, 1, 0.36, 1); } .hero-item-in { animation-name: heroSlideIn; animation-duration: 500ms; } .hero-item-out { animation-name: heroSlideOut; animation-duration: 250ms; }Then remove lines 216-231 from the component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/HeroSlider/index.tsx` around lines 216 - 231, The inline <style> block in the HeroSlider component is creating duplicate keyframe and class rules on each render; move the keyframes (heroSlideIn, heroSlideOut) and the .hero-item / .hero-item-in / .hero-item-out rules into your global stylesheet (e.g., src/styles/globals.css) and then remove the <style>{`...`}</style> block from the HeroSlider component (index.tsx) so the component simply relies on the global CSS classes; keep the class names unchanged so existing JSX that uses "hero-item", "hero-item-in", and "hero-item-out" continues to work.src/components/Layout/AmoledNavbar/index.tsx (1)
100-108: Inline<style>tag should be moved to globals.css.Similar to HeroSlider, this inline style block will be duplicated on each render. Move the
iconPopkeyframe animation toglobals.css.♻️ Move to globals.css
`@keyframes` iconPop { from { opacity: 0; transform: scale(0.6) rotate(-15deg); } to { opacity: 1; transform: scale(1) rotate(0deg); } } .icon-pop { animation: iconPop 250ms cubic-bezier(0.34, 1.56, 0.64, 1) both; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Layout/AmoledNavbar/index.tsx` around lines 100 - 108, Remove the inline <style> block in the AmoledNavbar component (index.tsx) and add the keyframes and .icon-pop rule to globals.css instead; specifically move the iconPop keyframe and .icon-pop animation declaration into globals.css, keep using the same className ("icon-pop") in the component markup, and delete the style template literal in the AmoledNavbar file (and remove any duplicated inline style in HeroSlider if present) so the animation is defined globally and not re-inserted on each render.src/components/RequestList/index.tsx (1)
276-276: Use semantic hover tokens to avoid theme drift.Line 276 still uses hard-coded
hover:!bg-gray-400, which can look off in AMOLED mode. Consider a semantic surface token for hover so this control stays theme-consistent.♻️ Proposed tweak
- className="z-40 mr-2 rounded-l-none border border-border-default !bg-input !px-3 !text-gray-500 hover:!bg-gray-400 hover:!text-white" + className="z-40 mr-2 rounded-l-none border border-border-default !bg-input !px-3 !text-gray-500 hover:!bg-surface-raised hover:!text-white"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RequestList/index.tsx` at line 276, The button/input class string in RequestList (the className that currently contains "hover:!bg-gray-400") uses a hard-coded color token which causes theme drift; replace the hard-coded hover utility with your design-system semantic hover token (e.g., "hover:!bg-surface-hover" or the project's equivalent token name) in the same className string so the control uses the theme surface hover style and remains consistent across light/dark/AMOLED modes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Common/ButtonWithDropdown/index.tsx`:
- Around line 35-40: The 'glass' variant branch in ButtonWithDropdown removes
the default focus outline but doesn't add a visible focus-visible indicator;
update the class lists built in the case 'glass' block for
styleClasses.mainButtonClasses and styleClasses.dropdownSideButtonClasses (and
the other two similar glass assignments at the other occurrences) to append
accessible focus-visible utility classes (e.g., focus-visible:ring-2
focus-visible:ring-white/40 focus-visible:ring-offset-2 or equivalent) so
keyboard focus is clearly visible while preserving the visual glass styling.
In `@src/components/Common/Dropdown/index.tsx`:
- Around line 28-30: The glass variant suppresses the default outline and needs
an explicit visible keyboard focus state: update the Dropdown component logic
that checks buttonType === 'glass' to append a focus-visible utility (e.g.,
focus-visible:outline, focus-visible:ring-2, focus-visible:ring-white/[.xx], or
focus-visible:ring-offset) to the class string used for the trigger and for
dropdown items (the code paths handling the trigger control and item controls
referenced around the same conditional), so keyboard users get a clear visible
ring/outline when focused while preserving the glass look.
In `@src/components/Discover/DiscoverMovies/index.tsx`:
- Around line 67-68: The hidden AMOLED backdrop logic must be gated by the
current theme to avoid fetching large images for users not using AMOLED; update
the effects and handlers that set backdropPath (and related logic that uses
pickedRef) to first check the theme preference (e.g., an isAmoled or theme value
from your theme context/prop) and only call setBackdropPath or initiate the
backdrop download when the theme indicates AMOLED/dark mode is active and the
user opted-in (pickedRef). Concretely, in the functions/effects that reference
backdropPath, setBackdropPath and pickedRef (and the same blocks currently
around lines where backdrop is set/cleared) add a guard like if (!isAmoled)
return so the hidden backdrop is never resolved/fetched for non-AMOLED themes.
In `@src/components/Discover/DiscoverTv/index.tsx`:
- Around line 53-55: The backdrop <img> elements in the DiscoverTv component are
still instantiated even when the AMOLED theme hides them, causing unnecessary
image loads; update the JSX to only create/render those <img> elements when the
active theme allows backdrops (e.g., check a theme flag from context or props
such as isAmoledTheme/useTheme) and when backdropPath is non-null—apply this
conditional gating where backdropPath/setBackdropPath is used and at the other
image spots referenced (around the blocks you noted at 68-75 and 86-102); ensure
you do not merely hide via CSS but short-circuit rendering (e.g., render
{allowBackdrop && backdropPath && <img ... />}), and avoid setting src until the
condition passes.
In `@src/components/Layout/AmoledNavbar/index.tsx`:
- Around line 70-77: The logout function currently calls revalidate() but
doesn't invalidate Layout's SWR caches for request/issue counts, so stale counts
can persist; update the logout handler (function logout) to import and call
SWR's global mutate to clear those keys (e.g., call
mutate('/api/v1/request/count') and mutate('/api/v1/issue/count') or a single
mutate() to clear all caches) immediately after a successful logout response and
before any redirect so Layout will reflect the logged-out state.
In `@src/components/MovieDetails/index.tsx`:
- Around line 1140-1143: The JSX contains a broken ternary and an unbraced
comment in the MovieDetails component; fix it by importing and calling the
existing useTheme hook (from src/hooks/useTheme.ts) inside the MovieDetails
component to obtain theme, use a conditional like theme === 'amoled-strix' as
the ternary condition to choose between the standard layout and the AMOLED
full-bleed layout (replace the stray ") : (" with ") ? (standardLayout) :
(amoledLayout)"), and ensure any inline JSX comment is wrapped in braces (e.g.,
{/* ... */}) so the JSX compiles correctly; identify the standard layout block
and the AMOLED block by the surrounding divs and className "relative bg-black"
to place them as the true/false branches.
In `@src/components/TitleCard/index.tsx`:
- Around line 385-387: The icon-only <Button> components lack accessible names;
update the Button instances for the watchlist, remove-watchlist and blocklist
controls (the components using props/onClick handlers onClickWatchlistBtn,
onClickRemoveWatchlistBtn, onClickBlocklistBtn and the one rendering <StarIcon>
etc.) to include an appropriate aria-label prop (e.g., aria-label="Add to
watchlist", "Remove from watchlist", "Block user") matching the native button
labels so screen readers get an accessible name.
---
Duplicate comments:
In `@src/components/TvDetails/index.tsx`:
- Around line 1364-1366: The JSX contains a broken ternary ") : (" without a
matching "?" and an unwrapped JSX comment; fix by importing and using useTheme
(e.g., call useTheme() and derive isAmoled via const isAmoled = theme ===
'amoled-strix') and then wrap the return in a proper ternary like return
isAmoled ? (/* AMOLED JSX */) : (/* standard JSX */); also convert the inline
comment on the AMOLED branch to a JSX comment using {/* ... */} so linting
passes; update references in this component (TvDetails/index.tsx) to use the new
isAmoled condition.
---
Nitpick comments:
In `@src/components/Common/SlideOver/index.tsx`:
- Line 74: The SlideOver container div in the SlideOver component currently
hard-codes the ring color (ring-gray-700); replace that class with the semantic
ring token used across the design system (e.g., ring-outline) so the component
follows theme tokens instead of a fixed color — update the class string in the
div inside the SlideOver component (the line with `flex h-full flex-col
rounded-lg shadow-xl ring-1 backdrop-blur bg-surface ring-gray-700`) to use the
semantic ring class (replace `ring-gray-700` with your project’s semantic ring
utility).
In `@src/components/Common/Table/index.tsx`:
- Line 9: Replace the unnecessary template literals used for static className
values on the Table component's tbody elements with plain string literals:
change className={`divide-y divide-border-default bg-surface`} (and the similar
occurrence at line 19) to use regular quoted strings so the JSX uses "divide-y
divide-border-default bg-surface" (and the corresponding static class string)
instead of backtick template syntax.
In `@src/components/Discover/HeroSlider/index.tsx`:
- Around line 216-231: The inline <style> block in the HeroSlider component is
creating duplicate keyframe and class rules on each render; move the keyframes
(heroSlideIn, heroSlideOut) and the .hero-item / .hero-item-in / .hero-item-out
rules into your global stylesheet (e.g., src/styles/globals.css) and then remove
the <style>{`...`}</style> block from the HeroSlider component (index.tsx) so
the component simply relies on the global CSS classes; keep the class names
unchanged so existing JSX that uses "hero-item", "hero-item-in", and
"hero-item-out" continues to work.
In `@src/components/Layout/AmoledNavbar/index.tsx`:
- Around line 100-108: Remove the inline <style> block in the AmoledNavbar
component (index.tsx) and add the keyframes and .icon-pop rule to globals.css
instead; specifically move the iconPop keyframe and .icon-pop animation
declaration into globals.css, keep using the same className ("icon-pop") in the
component markup, and delete the style template literal in the AmoledNavbar file
(and remove any duplicated inline style in HeroSlider if present) so the
animation is defined globally and not re-inserted on each render.
In `@src/components/MovieDetails/index.tsx`:
- Around line 1145-1171: Duplicate modal components (IssueModal,
ManageSlideOver, BlocklistModal) are rendered in both layout branches; hoist
them out of the conditional so they render once and use the same state/handlers.
Remove the duplicated instances inside the standard and AMOLED branches and
place a single set of IssueModal (props: show={showIssueModal}, onCancel={() =>
setShowIssueModal(false)}, mediaType="movie", tmdbId={data.id}), ManageSlideOver
(props: data={data}, mediaType="movie", show={showManager}, onClose={() => {
setShowManager(false); router.push({ pathname: router.pathname, query: {
movieId: router.query.movieId } }); }}, revalidate={() => revalidate()}), and
BlocklistModal (props: tmdbId={data.id}, type="movie",
show={showBlocklistModal}, onCancel={closeBlocklistModal},
onComplete={onClickHideItemBtn}, isUpdating={isBlocklistUpdating}) immediately
outside the conditional that chooses standard vs AMOLED layout so all branches
share these components and their state.
In `@src/components/RequestList/index.tsx`:
- Line 276: The button/input class string in RequestList (the className that
currently contains "hover:!bg-gray-400") uses a hard-coded color token which
causes theme drift; replace the hard-coded hover utility with your design-system
semantic hover token (e.g., "hover:!bg-surface-hover" or the project's
equivalent token name) in the same className string so the control uses the
theme surface hover style and remains consistent across light/dark/AMOLED modes.
In `@src/components/Setup/JellyfinSetup.tsx`:
- Around line 175-177: The span's className currently uses a template literal
without interpolation; change the className in the JSX span (the hostname prefix
span that renders {values.useSsl ? 'https://' : 'http://'}) from a template
literal to a plain string literal containing the classes (keep
border-border-default, bg-input, inline-flex, cursor-default, items-center,
rounded-l-md, border, border-r-0, px-3, text-gray-100, sm:text-sm) to simplify
the code.
In `@src/components/TitleCard/index.tsx`:
- Around line 488-490: The inline background gradient in TitleCard (component
TitleCard, JSX element with class/title-card-detail-link) uses hardcoded rgba
values; move that gradient into a CSS custom property (e.g.
--title-card-detail-gradient) defined in globals.css and provide theme-specific
overrides under [data-theme='amoled-strix'] so the semantic token system drives
colors, then replace the inline style in TitleCard with a reference to that CSS
variable. Ensure the CSS custom property is set to the same gradient string and
that the JSX removes hardcoded rgba values and uses the property instead.
In `@src/components/Toast/index.tsx`:
- Line 30: Replace the hardcoded tailwind class "ring-gray-500" with the
semantic ring token so the Toast stays theme-aware; update the class on the div
in src/components/Toast/index.tsx (the element with className starting
"pointer-events-auto w-full max-w-sm...") to use the semantic ring token (e.g.,
ring-ring) instead of ring-gray-500.
In `@src/components/UserProfile/index.tsx`:
- Line 156: Replace the hardcoded ring class used on the quota card div in the
UserProfile component with the semantic ring token used elsewhere so theme
appearance stays consistent; specifically, update the div that currently uses
`ring-gray-700` (and the equivalent non-restricted fallback cases referenced
near the other quota card blocks) to use the semantic token such as
`ring-ring-app/20` or `ring-border-default` so all quota card containers (the
div with classes including `overflow-hidden px-4 py-5 sm:p-6 rounded-lg
bg-surface shadow ring-1`) use the same theme-aware ring class.
In `@src/context/ThemeContext.tsx`:
- Around line 55-59: The setTheme callback should be defensive for SSR: keep
calling setThemeState(next) but wrap the client-only operations
(localStorage.setItem and document.documentElement.setAttribute) in a runtime
guard (e.g., check typeof window !== 'undefined' or typeof document !==
'undefined' and typeof localStorage !== 'undefined') so setTheme (the
useCallback that calls setThemeState, localStorage.setItem, and
document.documentElement.setAttribute) won't throw during server rendering;
update the setTheme body to perform the guard before accessing localStorage or
document.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19535b0b-8418-47f3-ace4-e9ef2ea457d9
📒 Files selected for processing (71)
src/components/Blocklist/index.tsxsrc/components/CollectionDetails/index.tsxsrc/components/Common/ButtonWithDropdown/index.tsxsrc/components/Common/Dropdown/index.tsxsrc/components/Common/ImageFader/index.tsxsrc/components/Common/Modal/index.tsxsrc/components/Common/PlayButton/index.tsxsrc/components/Common/SettingsTabs/index.tsxsrc/components/Common/SlideOver/index.tsxsrc/components/Common/Table/index.tsxsrc/components/Common/Tag/index.tsxsrc/components/Common/Tooltip/index.tsxsrc/components/Discover/DiscoverMovies/index.tsxsrc/components/Discover/DiscoverSliderEdit/index.tsxsrc/components/Discover/DiscoverTv/index.tsxsrc/components/Discover/HeroSlider/index.tsxsrc/components/Discover/Trending.tsxsrc/components/Discover/index.tsxsrc/components/IssueDetails/IssueComment/index.tsxsrc/components/IssueDetails/index.tsxsrc/components/IssueList/IssueItem/index.tsxsrc/components/IssueList/index.tsxsrc/components/IssueModal/CreateIssueModal/index.tsxsrc/components/Layout/AmoledNavbar/index.tsxsrc/components/Layout/PullToRefresh/index.tsxsrc/components/Layout/UserDropdown/index.tsxsrc/components/Layout/index.tsxsrc/components/Login/JellyfinLogin.tsxsrc/components/Login/LocalLogin.tsxsrc/components/Login/index.tsxsrc/components/MediaSlider/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/PersonDetails/index.tsxsrc/components/RegionSelector/index.tsxsrc/components/RequestBlock/index.tsxsrc/components/RequestButton/index.tsxsrc/components/RequestCard/index.tsxsrc/components/RequestList/RequestItem/index.tsxsrc/components/RequestList/index.tsxsrc/components/RequestModal/AdvancedRequester/index.tsxsrc/components/ResetPassword/RequestResetLink.tsxsrc/components/ResetPassword/index.tsxsrc/components/Settings/OverrideRule/OverrideRuleTiles.tsxsrc/components/Settings/SettingsAbout/Releases/index.tsxsrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsLogs/index.tsxsrc/components/Settings/SettingsMetadata.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/Settings/SettingsServices.tsxsrc/components/Setup/JellyfinSetup.tsxsrc/components/Setup/index.tsxsrc/components/Slider/index.tsxsrc/components/TitleCard/ErrorCard.tsxsrc/components/TitleCard/index.tsxsrc/components/Toast/index.tsxsrc/components/TvDetails/Season/index.tsxsrc/components/TvDetails/index.tsxsrc/components/UserList/index.tsxsrc/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsxsrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsWebPush/DeviceItem.tsxsrc/components/UserProfile/UserSettings/index.tsxsrc/components/UserProfile/index.tsxsrc/context/ThemeContext.tsxsrc/hooks/useTheme.tssrc/pages/_app.tsxsrc/pages/_document.tsxsrc/pages/profile/settings/appearance.tsxsrc/pages/users/[userId]/settings/appearance.tsxsrc/styles/globals.csstailwind.config.js
🚧 Files skipped from review as they are similar to previous changes (39)
- src/components/Common/PlayButton/index.tsx
- src/components/PersonDetails/index.tsx
- src/components/Discover/DiscoverSliderEdit/index.tsx
- src/components/IssueModal/CreateIssueModal/index.tsx
- src/components/Slider/index.tsx
- src/components/Settings/SettingsMetadata.tsx
- src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx
- src/components/RequestBlock/index.tsx
- src/components/Settings/SettingsAbout/Releases/index.tsx
- src/pages/users/[userId]/settings/appearance.tsx
- src/components/Layout/index.tsx
- src/pages/profile/settings/appearance.tsx
- src/components/Discover/Trending.tsx
- src/components/Setup/index.tsx
- src/components/TvDetails/Season/index.tsx
- src/components/Settings/SettingsJellyfin.tsx
- src/components/Settings/SettingsLogs/index.tsx
- src/components/Common/ImageFader/index.tsx
- src/components/Settings/OverrideRule/OverrideRuleTiles.tsx
- src/components/Layout/PullToRefresh/index.tsx
- src/components/Common/SettingsTabs/index.tsx
- src/components/IssueDetails/index.tsx
- src/components/RegionSelector/index.tsx
- src/components/UserList/index.tsx
- src/components/Layout/UserDropdown/index.tsx
- tailwind.config.js
- src/components/IssueDetails/IssueComment/index.tsx
- src/components/IssueList/IssueItem/index.tsx
- src/components/MediaSlider/index.tsx
- src/components/Common/Modal/index.tsx
- src/components/Common/Tooltip/index.tsx
- src/components/Login/JellyfinLogin.tsx
- src/components/Discover/index.tsx
- src/components/Settings/SettingsPlex.tsx
- src/components/Blocklist/index.tsx
- src/components/IssueList/index.tsx
- src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsWebPush/DeviceItem.tsx
- src/components/Common/Tag/index.tsx
- src/components/Settings/SettingsServices.tsx
- Fix critical broken ternary in MovieDetails and TvDetails (missing isAmoledTheme condition, unbraced JSX comments) - Add focus-visible ring to glass variant in ButtonWithDropdown and Dropdown - Gate AMOLED backdrop image in DiscoverMovies and DiscoverTv behind isAmoledTheme to prevent unnecessary image loads - Add aria-label to icon-only Button components in TitleCard
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/components/TvDetails/index.tsx (1)
1369-1371:⚠️ Potential issue | 🔴 CriticalFix invalid JSX in the ternary else branch.
At Line 1369, the
: (branch starts, then Line 1370 adds a standalone JSX comment before the<div>on Line 1371. That produces an invalid branch expression and matches the parse failures.🔧 Proposed fix
- ) : ( - {/* ── AMOLED full-bleed layout ── */} - <div className="relative bg-black"> + ) : ( + <div className="relative bg-black"> + {/* ── AMOLED full-bleed layout ── */}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` around lines 1369 - 1371, The JSX ternary else branch in the TvDetails component contains a standalone JSX comment between the opening parenthesis and the root element, causing a parse error; remove or move the JSX comment so the else branch directly returns the <div className="relative bg-black"> element (or wrap the comment inside the returned fragment/element) in the ternary expression used in the TvDetails component to restore valid JSX.src/components/MovieDetails/index.tsx (1)
1146-1148:⚠️ Potential issue | 🔴 CriticalThe else branch still contains invalid JSX structure.
At Line 1146 the ternary else branch starts, Line 1147 inserts a standalone JSX comment, then Line 1148 starts the
<div>. This creates the same parse break pattern as before.🔧 Proposed fix
- ) : ( - {/* ── AMOLED full-bleed layout ── */} - <div className="relative bg-black"> + ) : ( + <div className="relative bg-black"> + {/* ── AMOLED full-bleed layout ── */}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` around lines 1146 - 1148, The else branch of the ternary in MovieDetails (the branch beginning at the ternary else around the AMOLED full-bleed layout) contains a standalone JSX comment that breaks parsing; move the JSX comment so it is inside a JSX element (e.g., place the comment as the first child inside the <div className="relative bg-black">) or convert it to a JS/TS comment above the return/JSX expression, ensuring the else branch starts immediately with a valid JSX node (the <div> or a fragment) rather than a standalone comment.
🧹 Nitpick comments (4)
src/components/Discover/DiscoverMovies/index.tsx (1)
111-114: Potential duplicate heading accessibility concern.Both the
h1(line 111) andHeadercomponent (likely also anh1) are rendered to the DOM simultaneously, with visibility toggled via CSS. If the CSS usesdisplay: noneto hide the inactive heading, this is acceptable. However, if it usesvisibility: hiddenoropacity: 0, screen readers may announce both headings.Consider verifying the CSS ensures
display: nonefor the hidden variant, or conditionally render only one heading based onisAmoledThemeto eliminate the duplication entirely:♻️ Optional refactor to conditionally render single heading
<div className="mb-4 flex flex-col justify-between lg:flex-row lg:items-end"> - <h1 className="discover-movies-amoled-heading mt-5 text-4xl font-bold tracking-tight text-blue-300">{title}</h1> - <div className="discover-movies-standard-heading"> - <Header>{title}</Header> - </div> + {isAmoledTheme ? ( + <h1 className="mt-5 text-4xl font-bold tracking-tight text-blue-300">{title}</h1> + ) : ( + <Header>{title}</Header> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/DiscoverMovies/index.tsx` around lines 111 - 114, There are two headings rendered: the literal h1 element and the Header component (both using {title}), which can cause duplicate announcements for screen readers; update the DiscoverMovies component to render only one heading at a time by conditionally rendering the h1 or the Header based on the theme flag (e.g., isAmoledTheme) or ensure the hidden variant uses display: none in CSS; locate the JSX that contains the <h1 className="discover-movies-amoled-heading ...">{title}</h1> and the <Header>{title}</Header> and either wrap them in a conditional (render h1 when isAmoledTheme true else render Header) or change the CSS for discover-movies-amoled-heading / discover-movies-standard-heading to use display: none for the inactive state so screen readers won’t see both.src/components/TitleCard/index.tsx (3)
377-419: Addtype="button"to native button elements.The native
<button>elements (lines 378, 391, 409) lack thetype="button"attribute. While not strictly necessary outside forms, it's a defensive best practice to prevent unintended form submission if this component is ever nested inside a form.🛠️ Suggested fix
<button + type="button" aria-label={intl.formatMessage(messages.addToWatchList)} className="title-card-amoled-icon-btn ..." onClick={onClickWatchlistBtn} >Apply the same pattern to all native
<button>elements (lines 391, 409).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TitleCard/index.tsx` around lines 377 - 419, Native <button> elements in the TitleCard component are missing type="button", which can cause unexpected form submissions if nested in a form; update each native button (those using onClickWatchlistBtn, onClickDeleteWatchlistBtn, and the one that calls setShowBlocklistModal/EyeSlashIcon) to include type="button" so they behave as non-submitting buttons while leaving the existing Button component instances unchanged.
579-602: Addtype="button"to the AMOLED request button.Consistent with the other native buttons, add
type="button"to prevent potential form submission issues.🛠️ Suggested fix
<button + type="button" onClick={(e) => { e.preventDefault(); setShowRequestModal(true); }} className="title-card-amoled-request-btn ..." >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TitleCard/index.tsx` around lines 579 - 602, The AMOLED request native button (the <button> with className "title-card-amoled-request-btn" that calls setShowRequestModal in its onClick) is missing an explicit type, which can cause unintended form submissions; add type="button" to that button element so it behaves like the other native button and only triggers setShowRequestModal.
536-568: Consider using Tailwind'sline-clamputility instead of inline styles.The inline
WebkitLineClampstyles could be replaced with Tailwind'sline-clamp-*utilities for consistency and reduced inline style overhead.♻️ Example refactor for title
<h1 - className="whitespace-normal text-xl font-bold leading-tight" - style={{ - WebkitLineClamp: 3, - display: '-webkit-box', - overflow: 'hidden', - WebkitBoxOrient: 'vertical', - wordBreak: 'break-word', - }} + className="whitespace-normal text-xl font-bold leading-tight line-clamp-3 break-words" data-testid="title-card-title" >Apply similarly to the summary div with dynamic line clamp values (may require conditional class names).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TitleCard/index.tsx` around lines 536 - 568, Replace the inline WebkitLineClamp/display/WebkitBoxOrient/overflow styles in the TitleCard component: for the title h1 (data-testid="title-card-title") swap those inline styles for Tailwind's line-clamp utility (e.g., className includes "line-clamp-3"); for the summary div replace the computed inline WebkitLineClamp with conditional Tailwind classes (e.g., choose between "line-clamp-3" or "line-clamp-5" based on showRequestButton and currentStatus/MediaStatus checks) by building the className string accordingly; remove the redundant inline style properties (display, overflow, WebkitBoxOrient, wordBreak if already covered) and ensure the conditional logic that references showRequestButton, currentStatus and MediaStatus stays intact when computing the className for the summary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/MovieDetails/index.tsx`:
- Around line 1299-1305: The icon-only buttons that open modals (e.g., the
button invoking setShowBlocklistModal and rendering EyeSlashIcon and other
similar icon buttons) lack accessible names and explicit button types; update
each icon-only button to include a descriptive aria-label (e.g.,
aria-label="Open blocklist modal" or appropriate action text) and add
type="button" to prevent accidental form submission, ensuring you apply this to
the button that calls setShowBlocklistModal and the other icon-only buttons in
the same block.
- Around line 438-439: The useTheme() hook is being called after early return
paths in the MovieDetails component, violating React's rules of hooks; move the
call to useTheme() (and the derived isAmoledTheme) to the top level of the
component (before any conditional/early return logic) so hooks are always
invoked in the same order—specifically locate the useTheme() invocation and
isAmoledTheme constant in src/components/MovieDetails/index.tsx and relocate
them above the component's early return blocks.
In `@src/components/TvDetails/index.tsx`:
- Around line 1535-1540: Several icon-only native <button> elements (e.g., the
one that calls setShowBlocklistModal(true) and other similar onClick handlers
using EyeSlashIcon, etc., in TvDetails component) lack accessible names and
explicit button types; update each icon-only <button> (those using EyeSlashIcon
and other icons in this file) to include a descriptive aria-label (e.g.,
aria-label="Open blocklist modal") and add type="button" to prevent form
submission side-effects, ensuring each aria-label clearly describes the button
action and matches the associated onClick handler.
- Around line 469-470: The call to useTheme() (and the derived isAmoledTheme)
must be moved to the top of the TvDetails component before any conditional
return paths so hooks are always invoked in the same order; locate useTheme()
and isAmoledTheme in the TvDetails component and relocate them above the
earliest return statements (the early-loading/empty-result returns) so the hook
runs unconditionally on every render.
---
Duplicate comments:
In `@src/components/MovieDetails/index.tsx`:
- Around line 1146-1148: The else branch of the ternary in MovieDetails (the
branch beginning at the ternary else around the AMOLED full-bleed layout)
contains a standalone JSX comment that breaks parsing; move the JSX comment so
it is inside a JSX element (e.g., place the comment as the first child inside
the <div className="relative bg-black">) or convert it to a JS/TS comment above
the return/JSX expression, ensuring the else branch starts immediately with a
valid JSX node (the <div> or a fragment) rather than a standalone comment.
In `@src/components/TvDetails/index.tsx`:
- Around line 1369-1371: The JSX ternary else branch in the TvDetails component
contains a standalone JSX comment between the opening parenthesis and the root
element, causing a parse error; remove or move the JSX comment so the else
branch directly returns the <div className="relative bg-black"> element (or wrap
the comment inside the returned fragment/element) in the ternary expression used
in the TvDetails component to restore valid JSX.
---
Nitpick comments:
In `@src/components/Discover/DiscoverMovies/index.tsx`:
- Around line 111-114: There are two headings rendered: the literal h1 element
and the Header component (both using {title}), which can cause duplicate
announcements for screen readers; update the DiscoverMovies component to render
only one heading at a time by conditionally rendering the h1 or the Header based
on the theme flag (e.g., isAmoledTheme) or ensure the hidden variant uses
display: none in CSS; locate the JSX that contains the <h1
className="discover-movies-amoled-heading ...">{title}</h1> and the
<Header>{title}</Header> and either wrap them in a conditional (render h1 when
isAmoledTheme true else render Header) or change the CSS for
discover-movies-amoled-heading / discover-movies-standard-heading to use
display: none for the inactive state so screen readers won’t see both.
In `@src/components/TitleCard/index.tsx`:
- Around line 377-419: Native <button> elements in the TitleCard component are
missing type="button", which can cause unexpected form submissions if nested in
a form; update each native button (those using onClickWatchlistBtn,
onClickDeleteWatchlistBtn, and the one that calls
setShowBlocklistModal/EyeSlashIcon) to include type="button" so they behave as
non-submitting buttons while leaving the existing Button component instances
unchanged.
- Around line 579-602: The AMOLED request native button (the <button> with
className "title-card-amoled-request-btn" that calls setShowRequestModal in its
onClick) is missing an explicit type, which can cause unintended form
submissions; add type="button" to that button element so it behaves like the
other native button and only triggers setShowRequestModal.
- Around line 536-568: Replace the inline
WebkitLineClamp/display/WebkitBoxOrient/overflow styles in the TitleCard
component: for the title h1 (data-testid="title-card-title") swap those inline
styles for Tailwind's line-clamp utility (e.g., className includes
"line-clamp-3"); for the summary div replace the computed inline WebkitLineClamp
with conditional Tailwind classes (e.g., choose between "line-clamp-3" or
"line-clamp-5" based on showRequestButton and currentStatus/MediaStatus checks)
by building the className string accordingly; remove the redundant inline style
properties (display, overflow, WebkitBoxOrient, wordBreak if already covered)
and ensure the conditional logic that references showRequestButton,
currentStatus and MediaStatus stays intact when computing the className for the
summary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01c88019-c7f8-4e2d-b1eb-6558dc648394
📒 Files selected for processing (7)
src/components/Common/ButtonWithDropdown/index.tsxsrc/components/Common/Dropdown/index.tsxsrc/components/Discover/DiscoverMovies/index.tsxsrc/components/Discover/DiscoverTv/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/TitleCard/index.tsxsrc/components/TvDetails/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Discover/DiscoverTv/index.tsx
- src/components/Common/Dropdown/index.tsx
… buttons - Move useTheme() hook call before conditional returns in MovieDetails and TvDetails to comply with React rules of hooks - Add type=button and aria-label to all AMOLED icon-only buttons in MovieDetails (blocklist, watchlist add/remove) and TvDetails (blocklist, watchlist add/remove, report issue, manage series)
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/components/TvDetails/index.tsx (1)
1369-1372:⚠️ Potential issue | 🔴 CriticalFix malformed ternary branch JSX (compile blocker).
At Line 1369, the
: (branch begins with a JSX comment expression at Line 1370 followed by a<div>at **Line 1371`. This branch currently does not parse as a valid single JSX expression in this context, which is causing the parser failures reported at Lines 473/1371/2264.💡 Proposed fix
- ) : ( - {/* ── AMOLED full-bleed layout ── */} - <div className="relative bg-black"> + ) : ( + <> + {/* ── AMOLED full-bleed layout ── */} + <div className="relative bg-black"> ... - </div> + </div> + </> )}#!/bin/bash # Verify no ternary else-branch starts with a JSX comment expression in TvDetails. rg -nU '\)\s*:\s*\(\s*\{/\*' src/components/TvDetails/index.tsx # Expected result after fix: no matches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` around lines 1369 - 1372, The else branch of the ternary inside the TvDetails component is malformed because it starts with a JSX comment expression before the <div> (the branch after ") : ("), which breaks parsing; fix it by making the else branch a single valid JSX expression — either remove or move the JSX comment (/* ... */) outside the JSX expression or wrap the branch in a React fragment (<> ... </>) so the comment does not appear as the first token, ensuring the block that renders <div className="relative bg-black"> with <PageTitle title={data.name} /> is a valid single expression in the ternary.src/components/MovieDetails/index.tsx (1)
1146-1149:⚠️ Potential issue | 🔴 CriticalResolve invalid JSX in AMOLED ternary branch.
At Line 1146, the else branch starts with
: (, then a JSX comment at Line 1147 and a<div>at Line 1148. This branch shape is invalid in this context and is the root cause of the parse failures at Lines 442/1148/1719.💡 Proposed fix
- ) : ( - {/* ── AMOLED full-bleed layout ── */} - <div className="relative bg-black"> + ) : ( + <> + {/* ── AMOLED full-bleed layout ── */} + <div className="relative bg-black"> ... - </div> + </div> + </> )}#!/bin/bash # Verify no ternary else-branch starts with a JSX comment expression in MovieDetails. rg -nU '\)\s*:\s*\(\s*\{/\*' src/components/MovieDetails/index.tsx # Expected result after fix: no matches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` around lines 1146 - 1149, The JSX parse error is caused by a JSX comment placed directly at the start of the ternary else branch in the MovieDetails component (the branch that renders the AMOLED layout and returns <div className="relative bg-black"> and PageTitle); to fix it, remove or move the JSX comment out of the expression so the else branch begins with a valid JSX element (e.g., have the ternary’s else start with the <div> or wrap the comment above the return/JSX expression), ensuring the conditional expression in MovieDetails returns a single valid JSX node for the else branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/MovieDetails/index.tsx`:
- Around line 1146-1149: The JSX parse error is caused by a JSX comment placed
directly at the start of the ternary else branch in the MovieDetails component
(the branch that renders the AMOLED layout and returns <div className="relative
bg-black"> and PageTitle); to fix it, remove or move the JSX comment out of the
expression so the else branch begins with a valid JSX element (e.g., have the
ternary’s else start with the <div> or wrap the comment above the return/JSX
expression), ensuring the conditional expression in MovieDetails returns a
single valid JSX node for the else branch.
In `@src/components/TvDetails/index.tsx`:
- Around line 1369-1372: The else branch of the ternary inside the TvDetails
component is malformed because it starts with a JSX comment expression before
the <div> (the branch after ") : ("), which breaks parsing; fix it by making the
else branch a single valid JSX expression — either remove or move the JSX
comment (/* ... */) outside the JSX expression or wrap the branch in a React
fragment (<> ... </>) so the comment does not appear as the first token,
ensuring the block that renders <div className="relative bg-black"> with
<PageTitle title={data.name} /> is a valid single expression in the ternary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a8e00780-e2c0-42f7-a437-7120f1bcdb2f
📒 Files selected for processing (2)
src/components/MovieDetails/index.tsxsrc/components/TvDetails/index.tsx
A JSX comment expression followed by a div created two root elements in the ternary else branch, which is invalid. Remove the comments.
Author
ScreenshotsHere's a preview of the AMOLED-Strix theme across the main views: Home / Hero Slider — Cinematic full-bleed backdrop with poster, title, and request button. 
Discover — Series — Violet heading, backdrop blur at top, 2-column card grid. 
Discover — Movies — Same layout as Series with the Movies view. 
Title Cards — Glass-style card with watchlist star, rating badge, and inline Request button. 
Navigation Drawer — Pure black background, violet active state on the current page. 
Movie Detail — Backdrop hero with poster, metadata, and action buttons (blocklist, watchlist, trailer, request). 
|
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/MovieDetails/index.tsx (1)
1350-1373:⚠️ Potential issue | 🟠 MajorAdd accessible names to AMOLED icon-only issue/manage controls.
The AMOLED “Report issue” and “Manage movie” controls are icon-only and don’t expose an explicit accessible name in this branch. Please pass
aria-label(or an equivalent prop onButton) so screen readers can announce the action.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` around lines 1350 - 1373, The icon-only Buttons for reporting and managing a movie lack accessible names; update the Button in the ExclamationTriangleIcon block and the Button wrapping CogIcon to include an aria-label (or the Button's equivalent prop) using the corresponding localized strings (e.g., intl.formatMessage(messages.reportissue) for the report button and intl.formatMessage(messages.managemovie) for the manage button) so screen readers can announce the actions; ensure the props are added to the Button components that call setShowIssueModal(true) and setShowManager(true).
🧹 Nitpick comments (2)
src/components/TvDetails/index.tsx (1)
495-531: Consider extracting shared blocks to reduce branch drift risk.Both theme branches duplicate substantial functional JSX (modals, cast/recommendation/similar sliders, and several data sections). This will be expensive to maintain and easy to desync. Extract shared subcomponents and keep only layout-specific wrappers theme-conditional.
Also applies to: 1372-1408, 1325-1368, 2217-2261
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` around lines 495 - 531, The duplicated modal and manager JSX (BlocklistModal, IssueModal, RequestModal, ManageSlideOver and their props like tmdbId/data, show handlers, onComplete callbacks, isUpdating, revalidate, onClickHideItemBtn, closeBlocklistModal, router push) should be extracted into a single shared subcomponent (e.g., TvDetailsSharedModals) that accepts the necessary props (data/tmdbId, show flags, onCancel/onComplete handlers, isUpdating, revalidate, router handlers) and is imported into each theme branch; replace the duplicated modal blocks in the theme-specific files with this shared component so only layout/wrapper differences remain in the theme branches while all modal logic and callbacks are centralized.src/components/MovieDetails/index.tsx (1)
464-1145: Recommend extracting shared MovieDetails sections across theme branches.The two branch trees duplicate most functional sections (modals, metadata, cast, sliders, external links). Pulling shared logic into subcomponents would reduce regression risk and simplify future updates.
Also applies to: 1149-1716
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` around lines 464 - 1145, The MovieDetails file duplicates large shared UI sections across theme branches (modals, header, metadata, cast slider, MediaSlider blocks, ExternalLinkBlock) which should be extracted to reduce duplication; create reusable subcomponents (e.g., MovieDetailsHeader handling IssueModal/ManageSlideOver/BlocklistModal/StatusBadge/PlayButton/RequestButton and manager badge logic, MovieDetailsOverview for overview/crew/keywords/collection, MovieDetailsFacts for ratings/release/production/revenue/budget/external links using ExternalLinkBlock, and MovieDetailsCast for the cast Slider) and replace the duplicated blocks in src/components/MovieDetails/index.tsx with these components while preserving props like data, mediaLinks, ratingData, settings, locale, showManager, showIssueModal, showBlocklistModal, onClickWatchlistBtn, onClickDeleteWatchlistBtn, revalidate and permission checks (hasPermission) so behavior (e.g., StatusBadge, RequestButton, PlayButton, MediaSlider calls) remains identical across branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/MovieDetails/index.tsx`:
- Around line 1350-1373: The icon-only Buttons for reporting and managing a
movie lack accessible names; update the Button in the ExclamationTriangleIcon
block and the Button wrapping CogIcon to include an aria-label (or the Button's
equivalent prop) using the corresponding localized strings (e.g.,
intl.formatMessage(messages.reportissue) for the report button and
intl.formatMessage(messages.managemovie) for the manage button) so screen
readers can announce the actions; ensure the props are added to the Button
components that call setShowIssueModal(true) and setShowManager(true).
---
Nitpick comments:
In `@src/components/MovieDetails/index.tsx`:
- Around line 464-1145: The MovieDetails file duplicates large shared UI
sections across theme branches (modals, header, metadata, cast slider,
MediaSlider blocks, ExternalLinkBlock) which should be extracted to reduce
duplication; create reusable subcomponents (e.g., MovieDetailsHeader handling
IssueModal/ManageSlideOver/BlocklistModal/StatusBadge/PlayButton/RequestButton
and manager badge logic, MovieDetailsOverview for
overview/crew/keywords/collection, MovieDetailsFacts for
ratings/release/production/revenue/budget/external links using
ExternalLinkBlock, and MovieDetailsCast for the cast Slider) and replace the
duplicated blocks in src/components/MovieDetails/index.tsx with these components
while preserving props like data, mediaLinks, ratingData, settings, locale,
showManager, showIssueModal, showBlocklistModal, onClickWatchlistBtn,
onClickDeleteWatchlistBtn, revalidate and permission checks (hasPermission) so
behavior (e.g., StatusBadge, RequestButton, PlayButton, MediaSlider calls)
remains identical across branches.
In `@src/components/TvDetails/index.tsx`:
- Around line 495-531: The duplicated modal and manager JSX (BlocklistModal,
IssueModal, RequestModal, ManageSlideOver and their props like tmdbId/data, show
handlers, onComplete callbacks, isUpdating, revalidate, onClickHideItemBtn,
closeBlocklistModal, router push) should be extracted into a single shared
subcomponent (e.g., TvDetailsSharedModals) that accepts the necessary props
(data/tmdbId, show flags, onCancel/onComplete handlers, isUpdating, revalidate,
router handlers) and is imported into each theme branch; replace the duplicated
modal blocks in the theme-specific files with this shared component so only
layout/wrapper differences remain in the theme branches while all modal logic
and callbacks are centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a4d7b2a4-713c-42f9-98cf-abbbf1daffe9
📒 Files selected for processing (3)
src/components/MovieDetails/index.tsxsrc/components/TvDetails/index.tsxsrc/styles/globals.css
🚧 Files skipped from review as they are similar to previous changes (1)
- src/styles/globals.css
|
👋 @RAFSuNX, thank you for your contribution! |
Author
|
@gauthier-th hi, Sorry if I sound rude. I just wanted to ask whether the PR was rejected mainly because it seemed AI-generated, or if there were other issues with the code as well. I tried to guide the AI with strict rules and standards while generating the code, so I’d really appreciate any specific feedback on what should be improved. |
Member
You can't just write thousands of line of code using AI without supervision and expecting everything to be fine. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AI Assistance Disclosure
This PR was developed with Claude Code (Anthropic) as a coding assistant. All code was reviewed by me and tested against a live Seerr instance running both the default and AMOLED-Strix themes. I understand the implementation and can answer questions about it.
Summary
This PR introduces an AMOLED-Strix theme variant and a semantic CSS token system that makes all theme-aware components fully declarative — no per-component theme logic required.
What's included
1. Theme system foundation (
src/styles/globals.css,tailwind.config.js)**A new
[data-theme='amoled-strix']attribute on<html>remaps 7 semantic color tokens:--color-bg17 24 390 0 0--color-surface31 41 5514 14 14--color-surface-raised55 65 8124 24 24--color-input31 41 5510 10 10--color-border55 65 8136 36 36--color-border-muted52 52 5252 52 52--color-ring55 65 81255 255 255Tokens use bare RGB triplets (not
var()references) so Tailwind opacity modifiers likebg-surface/80work correctly.These wire into Tailwind as:
bg-app,bg-surface,bg-surface-raised,bg-input,border-border-default,border-border-muted,ring-ring-app.2. Appearance settings UI
A new "Appearance" settings page lets users switch between Default and AMOLED-Strix themes. The selection is persisted via a
data-themeattribute on<html>and stored in user preferences.3. AMOLED-Strix visual design
The AMOLED-Strix theme includes:
14,14,14) card surfaces — cards are visible but don't wash into the background4. Semantic token migration
All
isAmoledcomponent-level conditionals are replaced with semantic classes. Two approaches are used:isAmoled ? 'bg-black' : 'bg-gray-800'→bg-surface): handled via Tailwind semantic classes backed by CSS custom properties[data-theme='amoled-strix'] .stable-class { ... }rules inglobals.cssAfter migration:
isAmoledexists in exactly one file:ImageFader/index.tsx— a planned exception where the value is used to compute a CSS gradient string passed as an inline style (cannot be expressed as a class)useThemeis retained only inImageFaderand the theme switcher UI (UserAppearanceSettings)Test plan
Files changed
src/styles/globals.css— semantic token definitions + structural AMOLED CSS selectorstailwind.config.js— semantic color entriessrc/components/UserProfile/UserSettings/UserAppearanceSettings/— new appearance settings pagesrc/components/Layout/AmoledNavbar/— new AMOLED navbarsrc/components/Discover/HeroSlider/— new hero slider componentisAmoledconditionals replaced with semantic tokensSummary by CodeRabbit
New Features
Style
Accessibility