Frontend Overhaul#1292
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
This should be exported from the src/components/Inputs/index.ts file
There was a problem hiding this comment.
Switch should be imported from '@/src/components/Inputs'
There was a problem hiding this comment.
Strings should not be hard coded, this isn't the first place I see that happening
There was a problem hiding this comment.
The text color on some of these buttons isn't right
More hardcoded strings
There was a problem hiding this comment.
I don't like how so many features are gated behind "Advanced settings" that feels like a downgrade compared to the last version, Some users may be confused to the point of thinking features were removed.
There was a problem hiding this comment.
More hardcoded strings
There was a problem hiding this comment.
I'm not a fan of this at all, showing only a limited amount of features in the popup is a regression, If a user wants to quickly adjust a value for a feature they should be able to do it with the popup
There was a problem hiding this comment.
Basically same comment as for PopupView
There was a problem hiding this comment.
I'm not a big fan of the "Content filtering" title, some might thing it changes what comes up in recommendations or something. Most users aren't the brightest let's be honest
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/Settings/components/SettingsFooter.tsx (2)
27-33:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
clearDataleaves extension-owned state behind.This only overwrites configuration keys. Imported
state:*entries inbrowser.storage.localand the new localStorage-backed welcome/theme state from this stack remain untouched, so “clear data” is incomplete and stale behavior can survive the reset.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Settings/components/SettingsFooter.tsx` around lines 27 - 33, The clearData function currently only overwrites configuration keys; update clearData to also remove extension-owned state by (1) fetching all keys from browser.storage.local inside clearData, (2) removing any keys that start with the "state:" prefix and any specific keys used for welcome/theme state, (3) awaiting browser.storage.local.remove(...) for those keys (in addition to setting defaultConfiguration), and (4) clearing corresponding localStorage entries for welcome/theme before calling refreshSettings and addNotification so no extension-owned stale state survives the reset; reference clearData, defaultConfiguration, refreshSettings, and addNotification when making the changes.
27-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAwait
browser.storage.local.set()before reloading settings.
browser.storage.local.set(...)is async (Promise-returning) inwebextension-polyfill, and the code currently callsrefreshSettings()immediately, risking a read-after-write race and silently ignoring write failures.
clearData(27-33): await the storage write beforerefreshSettings().- Confirm reset handler (59-67): same.
Suggested fix
function clearData() { void (async () => { const userHasConfirmed = window.confirm(t((translations) => translations.pages.options.extras.clearData.confirmAlert)); if (userHasConfirmed) { - void browser.storage.local.set(defaultConfiguration); + await browser.storage.local.set(defaultConfiguration); await refreshSettings(); addNotification("success", (translations) => translations.pages.options.extras.clearData.allDataDeleted); } })(); } ... onClick={() => { void (async () => { const notificationToRemove = notifications.find((n) => n.action === "reset_settings"); if (notificationToRemove) { removeNotification(notificationToRemove); } - void browser.storage.local.set({ ...defaultConfiguration, ...{ rememberVolume: settings.rememberVolume } }); + await browser.storage.local.set({ ...defaultConfiguration, rememberVolume: settings.rememberVolume }); await refreshSettings(); addNotification("success", (translations) => translations.pages.options.notifications.success.saved); })(); }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Settings/components/SettingsFooter.tsx` around lines 27 - 33, The storage write is currently not awaited which can cause a read-after-write race and swallow failures; in clearData (function clearData) and in the reset confirmation handler (the confirm/reset handler around lines 59-67), await the Promise returned by browser.storage.local.set(...) before calling refreshSettings() or addNotification, and propagate or log any errors so refreshSettings() runs only after the write completes; locate calls to browser.storage.local.set, refreshSettings, and addNotification to update those flows.
♻️ Duplicate comments (4)
src/pages/popup/Popup.tsx (1)
8-16: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winQueryClient recreated on every render.
The QueryClient is instantiated inside the component body, so it's recreated on every render. This discards the cache and defeats React Query's memoization. Move the client to module scope or wrap it in
useState(() => new QueryClient(...)).♻️ Proposed fix
+const queryClient = new QueryClient({ + defaultOptions: { + queries: { + refetchInterval: 500, + refetchOnWindowFocus: true, + staleTime: 250 + } + } +}); + export default function Popup(): JSX.Element { - const client = new QueryClient({ - defaultOptions: { - queries: { - refetchInterval: 500, - refetchOnWindowFocus: true, - staleTime: 250 - } - } - }); return ( - <QueryClientProvider client={client}> + <QueryClientProvider client={queryClient}> <PopupView /> </QueryClientProvider> );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/popup/Popup.tsx` around lines 8 - 16, The QueryClient instance named client in Popup.tsx is being recreated on every render; move its creation out of the component (module scope) or initialize it once with useState to preserve React Query cache. Concretely, either declare a top-level const queryClient = new QueryClient({ defaultOptions: { queries: { refetchInterval: 500, refetchOnWindowFocus: true, staleTime: 250 } } }) and use that inside the Popup component, or inside the Popup component replace the direct new QueryClient(...) with const [client] = useState(() => new QueryClient({ defaultOptions: { queries: { refetchInterval: 500, refetchOnWindowFocus: true, staleTime: 250 } } })); then keep using client where currently referenced.src/components/Settings/components/WelcomeModal.tsx (1)
16-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove modal copy and recommended feature text into i18n resources.
This modal still hardcodes user-facing English strings, so non-English users will get mixed-language UI in first-time setup.
Also applies to: 93-100, 129-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Settings/components/WelcomeModal.tsx` around lines 16 - 24, The hardcoded user-facing strings in WelcomeModal.tsx (the array of feature objects and the modal title/body/button copy used in the WelcomeModal component) must be moved into i18n resources and replaced with translation lookups; locate the feature list object in WelcomeModal (the entries with keys like "hideShorts.enabled", "rememberVolume.enabled", "removeRedirect.enabled", etc.) and replace each description/label with a call to the project's i18n function (e.g., t('welcomeModal.features.hideShorts.description') and t('welcomeModal.features.hideShorts.label')), and also extract the modal title, descriptive paragraphs, and button labels (the hardcoded strings referenced around the other modal usages) into i18n keys and use t(...) there so all visible text uses localization keys. Ensure unique i18n keys per feature and per modal string and update any tests or snapshots that expect the English text.src/components/Settings/tabs/AppearanceTab.tsx (1)
17-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLocalize Appearance tab labels and button text.
The tab contains hardcoded UI strings (theme names, headings, titles, reset text), which breaks localization consistency.
Also applies to: 75-76, 99-100, 122-147
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Settings/tabs/AppearanceTab.tsx` around lines 17 - 34, The Appearance tab currently hardcodes UI strings in the color-label array and THEME_CARDS (and the headings/buttons at lines referenced), so replace the literal labels ("Blue", "Purple", "System", "Light", "Dark", "OLED Black", "Purple Night", "Ocean Blue", headings, reset text, etc.) with calls to the app's localization function (e.g., t('appearance.theme.blue') or similar) inside the AppearanceTab component; update the color constants and THEME_CARDS definitions to store translated labels (call t from within the component scope or map keys to t(...) at render time) and add corresponding i18n keys for each theme name and any button/heading text mentioned at lines 75-76, 99-100, and 122-147 so the UI uses localized strings instead of hardcoded English.src/components/Settings/SettingsGenerator.tsx (1)
38-61:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInternationalize section descriptions.
These hardcoded English descriptions break multilingual support. The project supports 16 locales, so all user-facing strings must use i18n translation keys like
SECTION_TITLE_FALLBACKSdoes.🌐 Recommended approach
Move these descriptions to locale files and use translation selectors:
-const SECTION_DESCRIPTIONS: Partial<Record<SettingsSectionId, string>> = { - automaticBehaviors: "Actions that trigger automatically when you load or watch a video", - buttonPlacement: "Configure where feature buttons appear in the player", +const SECTION_DESCRIPTIONS: Partial<Record<SettingsSectionId, TSelectFunc>> = { + automaticBehaviors: (t) => t((tr) => tr.settings.sections.automaticBehaviors.description), + buttonPlacement: (t) => t((tr) => tr.settings.sections.buttonPlacement.description), // ... etc for all 24 entries };Then update line 340 to evaluate the function:
- description={SECTION_DESCRIPTIONS[sectionId]} + description={SECTION_DESCRIPTIONS[sectionId]?.(t)}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Settings/SettingsGenerator.tsx` around lines 38 - 61, SECTION_DESCRIPTIONS contains hardcoded English strings — move each description into the locale files as translation keys (mirroring how SECTION_TITLE_FALLBACKS is handled), replace SECTION_DESCRIPTIONS with a function or object factory that accepts the i18n translator (e.g., getSectionDescriptions(t) or SECTION_DESCRIPTION_FALLBACKS) and returns translated strings, and update the consumer in SettingsGenerator (the same place SECTION_TITLE_FALLBACKS is evaluated) to call that function with the current t selector so the UI renders locale-specific descriptions.
🟡 Minor comments (17)
README.md-48-48 (1)
48-48:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix "You Tube" → "YouTube" in feature names.
Three feature names incorrectly spell YouTube as two words. The brand name is "YouTube" (one word).
✏️ Proposed typography fixes
-- **Open You Tube Settings On Hover**: Opens the YouTube settings menu when you hover over the settings button +- **Open YouTube Settings On Hover**: Opens the YouTube settings menu when you hover over the settings button-- **Remove You Tube /redirect URLs**: Replaces YouTube redirect by actual URLs, skipping redirect warning dialogue +- **Remove YouTube /redirect URLs**: Replaces YouTube redirect by actual URLs, skipping redirect warning dialogue-- **Shorten You Tube Video Share Link**: Shortens YouTube video share link (youtu.be) by stripping si/feature parameters from it +- **Shorten YouTube Video Share Link**: Shortens YouTube video share link (youtu.be) by stripping si/feature parameters from itAlso applies to: 50-50, 52-52
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 48, Locate the three feature name strings that currently spell "You Tube" (for example "**Open You Tube Settings On Hover**" and the two other similar feature lines) and change the typography to the correct brand name "YouTube" (e.g., "**Open YouTube Settings On Hover**"); update all three occurrences so the feature names consistently use "YouTube" as one word.src/components/Settings/components/ImportExportSection.tsx-155-184 (1)
155-184:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the new section copy.
These three strings are hardcoded English, so the new UI will regress in non-English locales even though the surrounding controls already use
t(...).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Settings/components/ImportExportSection.tsx` around lines 155 - 184, The Import & Export section has hardcoded English strings ("Import & Export" heading, "or drop a .json file", "saves as JSON") that must be localized; update the JSX in ImportExportSection.tsx to replace these literals with t(...) calls using appropriate translation keys (e.g. pages.options.extras.importExportSettings.heading, .importButton.hint orDropJson, .exportButton.hint savesAsJson) so the heading and the two small-span helper texts use the same i18n pattern as the existing button labels; ensure you add corresponding entries to the translation files.src/pages/popup/Popup.tsx-11-13 (1)
11-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winstaleTime shorter than refetchInterval creates a stale data window.
With
staleTime: 250andrefetchInterval: 500, queries become stale at 250ms but don't refetch until 500ms, leaving a 250ms window where the UI shows stale data. For a popup syncing with storage changes, consider settingstaleTime: 0or matching it torefetchIntervalto avoid displaying outdated toggle states.⚙️ Proposed fix
defaultOptions: { queries: { refetchInterval: 500, refetchOnWindowFocus: true, - staleTime: 250 + staleTime: 0 } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/popup/Popup.tsx` around lines 11 - 13, The query options in Popup.tsx currently set refetchInterval: 500 and staleTime: 250 which creates a 250ms window of stale UI; update the query configuration used in the popup (the object containing refetchInterval and staleTime) to either set staleTime: 0 or set staleTime to the same value as refetchInterval (500) so queries are not considered stale before the next background refetch and the toggle state never briefly shows outdated data.src/components/Settings/SettingsGenerator.tsx-343-343 (1)
343-343:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEmpty string fallback may cause accessibility issues.
If no title is resolved,
title=""renders a section with no semantic heading, which can confuse screen readers and breaks document outline.🛡️ Proposed fix
- title={storedSectionTitle ? storedSectionTitle(t) : ""} + title={storedSectionTitle?.(t) ?? sectionId}This ensures every section has at least an identifier as a fallback heading.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Settings/SettingsGenerator.tsx` at line 343, The title prop is being set to an empty string when storedSectionTitle resolves to falsy, which harms accessibility; update the code that sets title (the storedSectionTitle usage in SettingsGenerator) to provide a meaningful fallback (for example use the section's key/id or a translated generic like t('settings.section') or generate "Section: {sectionId}") so title is never empty, or alternatively render/associate a visually-hidden heading and reference it via aria-labelledby for the section.public/locales/ca-ES.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize newly added Catalan strings before merge.
Line 293 through Line 316 and Line 1010 through Line 1016 are still in English in
ca-ES. This will show untranslated UI in the new tabs/welcome/theme flow.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/ca-ES.json` around lines 292 - 316, Translate the untranslated English strings in the Catalan locale by replacing the keys under "welcome" (title, subtitle, getStarted, customize, recommendedLabel), the "tabs" keys (basic, advanced, appearance), and the "appearance" block (title, themePresets, accentColor, resetTheme and the nested "themes" values: system, light, dark, oled, purple, ocean) with proper Catalan text; apply the same translations to the duplicate appearance/theme block later in the file (the other occurrence of the same keys) so both places show Catalan UI copy.public/locales/cs-CZ.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize newly added Czech strings before merge.
Line 293 through Line 316 and Line 1010 through Line 1016 are English placeholders in
cs-CZ, so the new onboarding/tabs/section headers won’t be localized.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/cs-CZ.json` around lines 292 - 316, Strings under the "welcome" (welcome.title, welcome.subtitle, welcome.getStarted, welcome.customize, welcome.recommendedLabel), "tabs" (tabs.basic, tabs.advanced, tabs.appearance) and "appearance" (appearance.title, appearance.themePresets, appearance.accentColor, appearance.resetTheme and appearance.themes.system/light/dark/oled/purple/ocean) keys are still English in the cs-CZ locale; replace those English placeholders with proper Czech translations and ensure the same keys are translated in the other occurrence of these blocks so both instances are localized consistently.public/locales/es-ES.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the newly added Spanish entries.
Line 293 through Line 316 and Line 1010 through Line 1016 are English in
es-ES, so the new welcome/tabs/appearance and section labels are not translated.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/es-ES.json` around lines 292 - 316, The es-ES locale contains untranslated English strings for the JSON keys "welcome" (title, subtitle, getStarted, customize, recommendedLabel), "tabs" (basic, advanced, appearance), and "appearance" (title, themePresets, accentColor, resetTheme, and the "themes" entries system/light/dark/oled/purple/ocean); update those values to their Spanish translations in public/locales/es-ES.json (also ensure the duplicate block around the keys at the other occurrence is translated) so each value is the proper Spanish phrase for the corresponding key.public/locales/fr-FR.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTranslate newly added French locale strings.
Line 293 through Line 316 and Line 1010 through Line 1016 are English in
fr-FR, which introduces mixed-language UI in the new settings flow.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/fr-FR.json` around lines 292 - 316, The fr-FR locale contains English strings for the newly added settings UI; update the entries in public/locales/fr-FR.json for the welcome and appearance/tabs keys (specifically "welcome.title", "welcome.subtitle", "welcome.getStarted", "welcome.customize", "welcome.recommendedLabel", "tabs.basic", "tabs.advanced", "tabs.appearance", "appearance.title", "appearance.themePresets", "appearance.accentColor", "appearance.resetTheme" and the theme names under "appearance.themes" such as "system", "light", "dark", "oled", "purple", "ocean") to their proper French translations, and also fix the duplicate English block present elsewhere in the file (the other block of settings strings around the second occurrence) so all these keys are consistently translated to French.public/locales/de-DE.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTranslate the newly added German locale entries.
Line 293 through Line 316 and Line 1010 through Line 1016 are English in
de-DE, which causes mixed-language UI in the new settings experience.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/de-DE.json` around lines 292 - 316, Replace the English strings under the locale keys with their German translations: update "welcome.title" → "Willkommen bei YouTube Enhancer!", "welcome.subtitle" → "Wir haben einige empfehlenswerte Einstellungen für neue Nutzer vorgewählt. Du kannst sie jederzeit ändern.", "welcome.getStarted" → "Loslegen", "welcome.customize" → "Einstellungen anpassen", "welcome.recommendedLabel" → "Empfohlene Einstellungen"; update "tabs.basic" → "Allgemein", "tabs.advanced" → "Erweitert", "tabs.appearance" → "Darstellung"; update "appearance.title" → "Darstellung", "appearance.themePresets" → "Designvorgaben", "appearance.accentColor" → "Akzentfarbe", "appearance.resetTheme" → "Standard wiederherstellen", and translate the "appearance.themes" entries: "system" → "System", "light" → "Hell", "dark" → "Dunkel", "oled" → "OLED Schwarz", "purple" → "Lila Nacht", "ocean" → "Ozeanblau"; apply the same German translations for the duplicate keys mentioned around the other block so no English strings remain in de-DE.public/locales/pl-PL.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winProvide Polish translations for the newly added settings keys.
These newly introduced values are still English (
welcome,tabs,appearance, and new section titles), which creates mixed-language UI inpl-PL.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/pl-PL.json` around lines 292 - 316, Translate the newly added English JSON keys into Polish in public/locales/pl-PL.json: replace the "welcome" block values ("title","subtitle","getStarted","customize","recommendedLabel") with Polish equivalents, translate the "tabs" values ("basic","advanced","appearance") and all entries under the "appearance" block ("title","themePresets","accentColor","resetTheme" and the "themes" names "system","light","dark","oled","purple","ocean") so the pl-PL locale contains full Polish strings (also apply the same translations for the repeated block at lines referenced 1009-1016).public/locales/fa-IR.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the newly added settings strings for this locale.
The newly added
welcome,tabs,appearance, and new section titles are still English, which will surface mixed-language UI for Persian users. Please providefa-IRtranslations for these keys.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/fa-IR.json` around lines 292 - 316, Replace the English placeholders in this locale with Persian translations: localize the "welcome" object keys (title, subtitle, getStarted, customize, recommendedLabel), the "tabs" keys (basic, advanced, appearance), the "appearance" section keys (title, themePresets, accentColor, resetTheme) and the nested "themes" keys (system, light, dark, oled, purple, ocean) with appropriate fa-IR strings; also locate and update the duplicate set of these keys elsewhere in the same file to ensure no mixed-language UI remains.public/locales/ko-KR.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize new settings strings in this Korean locale file.
The newly added onboarding/theme/tab texts and section titles are English, causing mixed-language rendering for Korean users. Please provide
ko-KRtranslations for these keys.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/ko-KR.json` around lines 292 - 316, The strings under the keys welcome.*, tabs.*, appearance.* and appearance.themes.* are still in English; update their values in the ko-KR locale JSON to Korean equivalents (translate: welcome.title, welcome.subtitle, welcome.getStarted, welcome.customize, welcome.recommendedLabel; tabs.basic, tabs.advanced, tabs.appearance; appearance.title, appearance.themePresets, appearance.accentColor, appearance.resetTheme; and appearance.themes.system, .light, .dark, .oled, .purple, .ocean) so the onboarding, tab labels and theme presets render fully localized for Korean users.public/locales/ja-JP.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTranslate the newly introduced keys for Japanese UI consistency.
The added
welcome,tabs,appearance, and new section-title strings are English inja-JP, which introduces mixed-language UI. Please localize these entries.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/ja-JP.json` around lines 292 - 316, Several UI strings under the keys "welcome" (title, subtitle, getStarted, customize, recommendedLabel), "tabs" (basic, advanced, appearance), and "appearance" (title, themePresets, accentColor, resetTheme, themes.system/light/dark/oled/purple/ocean) are still in English in the ja-JP file; replace each of these values with appropriate Japanese translations (and do the same for the duplicate set referenced around keys at the other location noted) so the Japanese locale contains fully localized strings for those keys.public/locales/he-IL.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the newly added settings strings for this locale.
The added
welcome,tabs,appearance, and section-title entries are in English, so Hebrew users will see mixed-language UI. Please translate these keys inhe-IL.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/he-IL.json` around lines 292 - 316, Translate the newly added English strings for the locale keys under "welcome", "tabs", and "appearance" into Hebrew: replace "welcome.title", "welcome.subtitle", "welcome.getStarted", "welcome.customize", "welcome.recommendedLabel", "tabs.basic", "tabs.advanced", "tabs.appearance", "appearance.title", "appearance.themePresets", "appearance.accentColor", "appearance.resetTheme" and the "appearance.themes" entries ("system", "light", "dark", "oled", "purple", "ocean") with proper Hebrew translations so the UI is fully localized for he-IL; keep the same JSON keys and punctuation, ensure strings are valid JSON (escaped where necessary), and apply the same translation for the duplicate entries referenced in lines 1009-1016.public/locales/it-IT.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize newly added entries instead of shipping English fallback text.
In this Italian locale, the new
welcome/tabs/appearancestrings and the new section titles are still English. This causes mixed-language UX; please addit-ITtranslations for these keys.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/it-IT.json` around lines 292 - 316, The Italian locale currently contains English fallback text for the new localization keys—replace the English values under the "welcome" object (title, subtitle, getStarted, customize, recommendedLabel), the "tabs" keys (basic, advanced, appearance), and the "appearance" block (title, themePresets, accentColor, resetTheme and the "themes" entries: system, light, dark, oled, purple, ocean) with proper it-IT translations; also locate the other duplicate occurrence of these keys and apply the same Italian translations so there is no mixed-language UX.public/locales/pt-BR.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTranslate new onboarding/theme/tab strings for pt-BR.
The new
welcome,tabs,appearance, and section-title values are still in English, which causes mixed-language UI in Brazilian Portuguese.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/pt-BR.json` around lines 292 - 316, The UI strings under the localization keys are still in English; translate every value under the "welcome" keys (welcome.title, welcome.subtitle, welcome.getStarted, welcome.customize, welcome.recommendedLabel), the "tabs" keys (tabs.basic, tabs.advanced, tabs.appearance), and the "appearance" keys (appearance.title, appearance.themePresets, appearance.accentColor, appearance.resetTheme and appearance.themes.system/light/dark/oled/purple/ocean) into Brazilian Portuguese (pt-BR) with correct accents/terminology, and make the same translations for the duplicate occurrence of these keys elsewhere in the file so both blocks match. Ensure the keys remain unchanged and only the string values are replaced.public/locales/nl-NL.json-292-316 (1)
292-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTranslate newly added locale keys to avoid mixed-language UX.
In
nl-NL, the newwelcome/tabs/appearanceentries and new section titles are still English. Please localize these values into Dutch.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/nl-NL.json` around lines 292 - 316, Translate the English strings for the newly added locale keys into Dutch: update the "welcome" object (title, subtitle, getStarted, customize, recommendedLabel), the "tabs" keys (basic, advanced, appearance), the "appearance" keys (title, themePresets, accentColor, resetTheme) and the nested "themes" entries (system, light, dark, oled, purple, ocean) with proper Dutch text while keeping the JSON keys unchanged and quoted; also apply the same translations to the other identical locale block elsewhere in the file to avoid mixed-language UX.
🧹 Nitpick comments (15)
src/pages/popup/PopupView.tsx (4)
94-108: ⚖️ Poor tradeoffInline switch implementation instead of reusing Switch component.
This layer depends on the Switch component introduced in an earlier layer, but the code implements an inline switch here. Reusing the
Switchcomponent would ensure consistent behavior and styling with the main Settings UI.Note: The inline implementation is functional and visually appropriate for the popup context. Consider reusing
Switchonly if strict UI consistency across all surfaces is a project requirement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/popup/PopupView.tsx` around lines 94 - 108, This inline switch in PopupView.tsx duplicates UI/behavior instead of reusing the existing Switch component; replace the button/span block with the shared Switch component (import Switch if missing) and wire it to the same props: pass checked={isOn}, onChange={(newVal) => toggleMutation.mutate({ featureKey: feature.key, newValue: newVal })} and any aria/role props (aria-checked, role="switch") and className overrides if needed so the Switch handles animation/styling consistently across the app.
36-36: 💤 Low valueDuplicated feature key parsing logic.
The
featureKey.split(".")pattern appears at lines 36 and 122. Extract this to a helper function or validate the format once to reduce duplication.♻️ Proposed refactor
+function parseFeatureKey(key: string): [string, string] { + const parts = key.split("."); + if (parts.length !== 2) throw new Error(`Invalid feature key format: ${key}`); + return parts as [string, string]; +} + const toggleMutation = useMutation({ mutationFn: async ({ featureKey, newValue }: { featureKey: string; newValue: boolean }) => { - const [featureName, settingName] = featureKey.split("."); + const [featureName, settingName] = parseFeatureKey(featureKey);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/popup/PopupView.tsx` at line 36, Extract the duplicated parsing logic featureKey.split(".") into a reusable helper (e.g., parseFeatureKey) and replace both occurrences in PopupView with calls to that helper; the helper should accept featureKey, validate the format (ensure it contains exactly one "."), return { featureName, settingName } (or throw/return null and handle errors where used), and update any code that currently does const [featureName, settingName] = featureKey.split(".") to use the new parseFeatureKey function.
118-118: 💤 Low valueUnnecessary Set creation.
Creating a Set and immediately spreading it to an array is redundant here. Since
QUICK_FEATURESis aconstarray with a known structure, just map directly.♻️ Proposed fix
- const featureKeys = [...new Set(QUICK_FEATURES.map((f) => f.key.split(".")[0]))]; + const featureKeys = QUICK_FEATURES.map((f) => f.key.split(".")[0]);Note: If duplicate feature names are a concern, retain the Set.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/popup/PopupView.tsx` at line 118, Replace the unnecessary Set usage when deriving featureKeys: instead of creating a new Set and spreading it, directly map QUICK_FEATURES to extract the prefix (use QUICK_FEATURES and featureKeys to locate the line) — e.g., set featureKeys to the mapped array of f.key.split(".")[0]; if deduplication is required keep the Set approach but otherwise remove it for simplicity.
30-31: 💤 Low valueQuery options duplicate parent defaults.
The
refetchInterval: 500andstaleTime: 250specified here are identical to the QueryClient defaults inPopup.tsx(lines 11, 13). Remove these redundant options to inherit from the parent configuration.♻️ Proposed fix
const { data: quickSettings } = useQuery({ queryFn: fetchQuickSettings, - queryKey: ["popup-settings"], - refetchInterval: 500, - staleTime: 250 + queryKey: ["popup-settings"] });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/popup/PopupView.tsx` around lines 30 - 31, In PopupView.tsx remove the redundant query options refetchInterval: 500 and staleTime: 250 from the query configuration so the component (PopupView) inherits the parent QueryClient defaults defined in Popup.tsx; locate the query invocation or options object used by PopupView (the place where refetchInterval and staleTime are currently set) and delete those two properties.src/components/Settings/sections/LanguageSettings.tsx (1)
37-40: 💤 Low valueConsider extracting the repeated className pattern.
The className string
"mb-3 break-inside-avoid rounded-xl bg-[var(--card-bg)] p-2 shadow-sm"appears identically in ButtonPlacement, FeatureMenuOpenType, and LanguageSettings. Consider extracting it to a shared constant for maintainability.♻️ Example refactor
Create a shared constant:
// In a shared constants file or at the top of a common module export const SETTING_SECTION_CARD_CLASSNAME = "mb-3 break-inside-avoid rounded-xl bg-[var(--card-bg)] p-2 shadow-sm";Then use it in each component:
<SettingSection - className="mb-3 break-inside-avoid rounded-xl bg-[var(--card-bg)] p-2 shadow-sm" + className={SETTING_SECTION_CARD_CLASSNAME} title={t((translations) => translations.pages.options.extras.language.title)} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Settings/sections/LanguageSettings.tsx` around lines 37 - 40, Extract the repeated className string used on SettingSection into a shared constant (e.g., SETTING_SECTION_CARD_CLASSNAME) and replace the literal in LanguageSettings, ButtonPlacement, and FeatureMenuOpenType with that constant; create the constant in a common module (or at top of a shared file), export it, and import it where each component (LanguageSettings, ButtonPlacement, FeatureMenuOpenType) uses the SettingSection component so all three reference the single exported constant instead of repeating the string.src/components/Settings/sections/YouTubeDataApiKey.tsx (1)
15-18: ⚡ Quick winConsider extracting the repeated className to a shared constant.
The same className string appears in both this file and
OnScreenDisplay.tsx(and per the stack context, likely inButtonPlacement.tsx,FeatureMenuOpenType.tsx, andLanguageSettings.tsxas well). Extracting it to a shared constant would ensure consistency and simplify future styling updates.♻️ Proposed refactor
Create a new file
src/components/Settings/sections/constants.ts:export const SECTION_CARD_CLASSNAME = "mb-3 break-inside-avoid rounded-xl bg-[var(--card-bg)] p-2 shadow-sm";Then import and use it in each section file:
+import { SECTION_CARD_CLASSNAME } from "./constants"; + export default function YouTubeDataApiKeySection() { // ... return ( <SettingSection - className="mb-3 break-inside-avoid rounded-xl bg-[var(--card-bg)] p-2 shadow-sm" + className={SECTION_CARD_CLASSNAME} title={t((translations) => translations.pages.options.extras.youtubeDataApiV3Key.title)} >Apply this pattern to all 5 section files mentioned in the stack context.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Settings/sections/YouTubeDataApiKey.tsx` around lines 15 - 18, Extract the repeated className used on SettingSection into a shared constant (e.g. SECTION_CARD_CLASSNAME) in a new module (suggested: src/components/Settings/sections/constants.ts) and replace the inline className prop in YouTubeDataApiKey's SettingSection with that constant; do the same replacement in the other files mentioned (OnScreenDisplay.tsx, ButtonPlacement.tsx, FeatureMenuOpenType.tsx, LanguageSettings.tsx) so they import SECTION_CARD_CLASSNAME and pass it to SettingSection instead of duplicating the string.src/components/Settings/sections/OnScreenDisplay.tsx (1)
114-117: Confirm--card-bgis defined for theming
bg-[var(--card-bg)]is backed bysrc/components/Settings/Settings.css, where--card-bgis defined for:rootand theme variants (light/dark +[data-theme="..."]).
RepeatedclassName="... bg-[var(--card-bg)] ..."across multiple settings sections could be extracted for consistency if you want.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Settings/sections/OnScreenDisplay.tsx` around lines 114 - 117, Verify that Settings.css defines --card-bg on :root and in each theme variant, then replace repeated inline usages of bg-[var(--card-bg)] on SettingSection components with a single shared utility/class to ensure consistency: update Settings.css to include a semantic class (e.g., .settings-card-bg) that applies background: var(--card-bg) and refactor SettingSection instances to use that class in their className instead of duplicating bg-[var(--card-bg)]; reference SettingSection, className, Settings.css and the --card-bg CSS variable when making the change.public/locales/zh-CN.json (1)
292-316: 🏗️ Heavy liftLocalize the newly added
zh-CNstrings.These keys are English-only (welcome/tabs/appearance and new section titles), which creates inconsistent Simplified Chinese UX in the updated settings interface.
Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/zh-CN.json` around lines 292 - 316, The zh-CN locale file is missing Chinese translations for the new keys under "welcome", "tabs", and "appearance" (including nested "themes" keys like "system","light","dark","oled","purple","ocean") — update public/locales/zh-CN.json by replacing the English strings for "welcome" (title, subtitle, getStarted, customize, recommendedLabel), "tabs" (basic, advanced, appearance), and "appearance" (title, themePresets, accentColor, resetTheme, and each theme name) with appropriate Simplified Chinese translations so the settings UI is fully localized.public/locales/vi-VN.json (1)
292-316: 🏗️ Heavy liftTranslate the new entries in
vi-VNto avoid mixed-language UI.The added welcome/tabs/appearance strings and new section titles are currently English and should be localized for Vietnamese users.
Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/vi-VN.json` around lines 292 - 316, The new localization entries under the "welcome", "tabs", and "appearance" keys (including nested keys like "getStarted", "customize", "recommendedLabel", "basic", "advanced", "appearance", "title", "themePresets", "accentColor", "resetTheme", and the "themes" variants "system", "light", "dark", "oled", "purple", "ocean") are still in English; replace them with Vietnamese translations so the UI is fully localized (translate each string to Vietnamese, keeping key names unchanged and preserving punctuation/casing). Ensure the same change is applied to the other occurrences referenced (lines 1009-1016) so no English remains.public/locales/hi-IN.json (1)
292-316: 🏗️ Heavy liftAdd Hindi translations for the newly added locale keys.
These new strings are English in
hi-IN(welcome modal, tabs, appearance labels, and section titles), causing a mixed-language experience.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/hi-IN.json` around lines 292 - 316, The hi-IN locale file contains English strings under the "welcome", "tabs", and "appearance" objects (e.g., welcome.title, welcome.subtitle, welcome.getStarted, welcome.customize, welcome.recommendedLabel, tabs.basic, tabs.advanced, tabs.appearance, appearance.title, appearance.themePresets, appearance.accentColor, appearance.resetTheme, appearance.themes.system/light/dark/oled/purple/ocean); replace these English values with proper Hindi translations for each key so the hi-IN file is fully localized (update the string values for those keys and ensure consistency for the additional occurrences mentioned at lines 1009-1016).public/locales/ru-RU.json (1)
292-316: 🏗️ Heavy liftLocalize newly added strings in the Russian locale file.
These new entries are English-only, so Russian users will see mixed-language UI in the welcome flow, tab labels, appearance settings, and new section headers. Please translate these keys in
ru-RUto keep locale consistency.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/ru-RU.json` around lines 292 - 316, Translate the newly added English strings under the "welcome", "tabs", and "appearance" keys into Russian in the ru-RU locale: provide Russian equivalents for welcome.title, welcome.subtitle, welcome.getStarted, welcome.customize, welcome.recommendedLabel, tabs.basic, tabs.advanced, tabs.appearance, appearance.title, appearance.themePresets, appearance.accentColor, appearance.resetTheme, and all entries under appearance.themes (system, light, dark, oled, purple, ocean); also apply the same translations to the other identical block of keys present elsewhere in the file to ensure consistency across the welcome flow and appearance settings.public/locales/tr-TR.json (1)
292-316: 🏗️ Heavy liftProvide Turkish translations for the newly introduced locale keys.
The new welcome/tabs/appearance labels and section headers are English-only in
tr-TR, causing inconsistent localized UX.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/tr-TR.json` around lines 292 - 316, The Turkish locale file is missing translations for the new keys; update the JSON by replacing the English strings for welcome.title, welcome.subtitle, welcome.getStarted, welcome.customize, welcome.recommendedLabel, tabs.basic, tabs.advanced, tabs.appearance, appearance.title, appearance.themePresets, appearance.accentColor, appearance.resetTheme and the appearance.themes entries (system, light, dark, oled, purple, ocean) with appropriate Turkish translations so the UI is fully localized (also apply the same translations where the duplicate block appears around the other occurrence).public/locales/zh-TW.json (1)
292-316: 🏗️ Heavy liftTranslate the new keys in
zh-TWfor locale consistency.The newly introduced welcome/tabs/appearance entries and section headers are English; this will show mixed-language UI for Traditional Chinese users.
Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/zh-TW.json` around lines 292 - 316, Translate the English values for the new locale keys into Traditional Chinese: replace welcome.title, welcome.subtitle, welcome.getStarted, welcome.customize, welcome.recommendedLabel, tabs.basic, tabs.advanced, tabs.appearance, appearance.title, appearance.themePresets, appearance.accentColor, appearance.resetTheme and all appearance.themes entries (system, light, dark, oled, purple, ocean) with appropriate zh-TW strings so the UI is fully localized; make the same replacements for the other identical block of keys introduced elsewhere in the file.public/locales/uk-UA.json (1)
292-316: 🏗️ Heavy liftAdd Ukrainian translations for the new settings strings.
These newly added keys are English in
uk-UA, which introduces mixed-language content in the welcome modal, tabs, appearance tab, and section grouping titles.Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/uk-UA.json` around lines 292 - 316, The uk-UA locale contains English strings for the new UI keys; replace them with Ukrainian translations for the welcome modal and appearance/tabs keys. Update the values for "welcome.title", "welcome.subtitle", "welcome.getStarted", "welcome.customize", "welcome.recommendedLabel", "tabs.basic", "tabs.advanced", "tabs.appearance", "appearance.title", "appearance.themePresets", "appearance.accentColor", "appearance.resetTheme" and each "appearance.themes" entry ("system","light","dark","oled","purple","ocean") to their proper Ukrainian equivalents (also apply same translations where the same keys appear later in the file around the other occurrence mentioned). Ensure whitespace/JSON formatting remains valid.public/locales/sv-SE.json (1)
292-316: 🏗️ Heavy liftTranslate the new
sv-SEkeys instead of shipping English text.The added welcome/tabs/appearance strings and new settings section titles are currently English, which introduces mixed-language UI for Swedish users.
Also applies to: 1009-1016
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/locales/sv-SE.json` around lines 292 - 316, Replace the English values with Swedish translations for the new localization keys: welcome.title, welcome.subtitle, welcome.getStarted, welcome.customize, welcome.recommendedLabel, tabs.basic, tabs.advanced, tabs.appearance, appearance.title, appearance.themePresets, appearance.accentColor, appearance.resetTheme and the appearance.themes entries (system, light, dark, oled, purple, ocean); ensure the string values are proper Swedish phrases and preserve JSON quoting/commas and key names; also apply the same Swedish translations to the duplicate appearance/theme keys located in the other block referenced (the keys around the other appearance section mentioned in the review).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 372d7733-6451-4341-85f4-a9eeee1afec2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (77)
README.mdpublic/locales/ca-ES.jsonpublic/locales/cs-CZ.jsonpublic/locales/de-DE.jsonpublic/locales/en-GB.jsonpublic/locales/en-US.jsonpublic/locales/en-US.json.d.tspublic/locales/es-ES.jsonpublic/locales/fa-IR.jsonpublic/locales/fr-FR.jsonpublic/locales/he-IL.jsonpublic/locales/hi-IN.jsonpublic/locales/it-IT.jsonpublic/locales/ja-JP.jsonpublic/locales/ko-KR.jsonpublic/locales/nl-NL.jsonpublic/locales/pl-PL.jsonpublic/locales/pt-BR.jsonpublic/locales/ru-RU.jsonpublic/locales/sv-SE.jsonpublic/locales/tr-TR.jsonpublic/locales/uk-UA.jsonpublic/locales/vi-VN.jsonpublic/locales/zh-CN.jsonpublic/locales/zh-TW.jsonsrc/components/Inputs/Switch/Switch.tsxsrc/components/Settings/Settings.csssrc/components/Settings/Settings.tsxsrc/components/Settings/SettingsGenerator.tsxsrc/components/Settings/components/ImportExportSection.tsxsrc/components/Settings/components/Setting.tsxsrc/components/Settings/components/SettingSection.tsxsrc/components/Settings/components/SettingTitle.tsxsrc/components/Settings/components/SettingsFooter.tsxsrc/components/Settings/components/TabBar.tsxsrc/components/Settings/components/WelcomeModal.tsxsrc/components/Settings/sections/ButtonPlacement.tsxsrc/components/Settings/sections/FeatureMenuOpenType.tsxsrc/components/Settings/sections/LanguageSettings.tsxsrc/components/Settings/sections/OnScreenDisplay.tsxsrc/components/Settings/sections/YouTubeDataApiKey.tsxsrc/components/Settings/tabs/AppearanceTab.tsxsrc/features/_registry/featureMetadataRegistry.tssrc/features/automaticTheaterMode/index.metadata.tssrc/features/automaticallyDisableAmbientMode/index.metadata.tssrc/features/automaticallyDisableAutoPlay/index.metadata.tssrc/features/automaticallyDisableClosedCaptions/index.metadata.tssrc/features/automaticallyEnableClosedCaptions/index.metadata.tssrc/features/automaticallyMaximizePlayer/index.metadata.tssrc/features/automaticallyShowMoreVideosOnEndScreen/index.metadata.tssrc/features/blockNumberKeySeeking/index.metadata.tssrc/features/defaultToOriginalAudioTrack/index.metadata.tssrc/features/hideArtificialIntelligenceSummary/index.metadata.tssrc/features/hideEndScreenCards/index.metadata.tssrc/features/hideEndScreenCardsButton/index.metadata.tssrc/features/hideLiveStreamChat/index.metadata.tssrc/features/hideMembersOnlyVideos/index.metadata.tssrc/features/hideOfficialArtistVideosFromHomePage/index.metadata.tssrc/features/hidePaidPromotionBanner/index.metadata.tssrc/features/hidePlayables/index.metadata.tssrc/features/hidePlaylistRecommendationsFromHomePage/index.metadata.tssrc/features/hidePosts/index.metadata.tssrc/features/hideScrollBar/index.metadata.tssrc/features/hideSidebarRecommendedVideos/index.metadata.tssrc/features/hideTranslateComment/index.metadata.tssrc/features/monoToStereo/index.metadata.tssrc/features/pauseBackgroundPlayers/index.metadata.tssrc/features/rememberVolume/index.metadata.tssrc/features/restoreFullscreenScrolling/index.metadata.tssrc/features/timestampPeek/index.metadata.tssrc/hooks/useSectionTitle/context.tssrc/hooks/useSectionTitle/provider.tsxsrc/pages/options/index.csssrc/pages/popup/Popup.tsxsrc/pages/popup/PopupView.tsxsrc/pages/popup/index.csssrc/utils/uiTheme.ts
| import { useSettings } from "../../Settings/Settings"; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Decouple Switch from SettingsContext to prevent runtime coupling.
Switch is in shared inputs but depends on useSettings(). Reusing it outside the settings provider will throw and break rendering. Keep layout direction local/CSS-based instead of context-coupled.
Proposed refactor
-import { useSettings } from "../../Settings/Settings";
@@
- const { direction } = useSettings();
const id = useId();
@@
- <div
- className={cn("min-w-0 max-w-[400px]", {
- "ml-2": direction === "ltr",
- "mr-2": direction === "rtl"
- })}
- >
+ <div className="min-w-0 max-w-[400px]" style={{ marginInlineStart: "0.5rem" }}>Also applies to: 18-19, 41-44
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Inputs/Switch/Switch.tsx` around lines 5 - 6, The Switch
component currently imports and calls useSettings (creating a runtime dependency
on SettingsContext) — remove that import/usage and decouple layout from context:
modify the Switch component to accept a prop (e.g., layoutDirection or
orientation) or rely solely on CSS classes for row/column layout, update its
prop types/defaultProps accordingly, and remove any conditional logic that reads
settings (references: Switch component and useSettings). Ensure callers that
relied on the context either pass the new prop or let the component use its
default styling, and remove the SettingsContext coupling in the code paths
highlighted (formerly lines 18-19, 41-44).
| export type SwitchProps = { | ||
| checked: boolean; | ||
| className?: string; | ||
| disabled?: boolean; | ||
| disabledReason?: string; | ||
| label: string; | ||
| onChange: (event: ChangeEvent<HTMLInputElement>) => void; | ||
| title: string; | ||
| }; |
There was a problem hiding this comment.
Require an accessible name for every switch control.
The checkbox can render without an accessible name when label is empty. Add an explicit ariaLabel prop and wire it to the <input>.
Proposed fix
export type SwitchProps = {
+ ariaLabel?: string;
checked: boolean;
@@
-const Switch: React.FC<SwitchProps> = ({ checked, className, disabled = false, disabledReason, label, onChange, title }) => {
+const Switch: React.FC<SwitchProps> = ({ ariaLabel, checked, className, disabled = false, disabledReason, label, onChange, title }) => {
@@
- <input checked={checked} className="peer sr-only" disabled={disabled} id={id} onChange={onChange} type="checkbox" />
+ <input
+ aria-label={ariaLabel ?? label}
+ checked={checked}
+ className="peer sr-only"
+ disabled={disabled}
+ id={id}
+ onChange={onChange}
+ type="checkbox"
+ />Also applies to: 23-24, 46-48
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Inputs/Switch/Switch.tsx` around lines 7 - 15, The Switch
component can render without an accessible name when label is empty—add an
optional ariaLabel prop to SwitchProps (e.g., ariaLabel?: string) and wire it to
the underlying <input> element inside the Switch component so the input receives
aria-label={ariaLabel ?? label}; update any places where Switch is used to allow
passing ariaLabel when no visible label is present. Ensure the prop name is
SwitchProps.ariaLabel and that the input element uses that fallback logic.
| const [conflicts, setConflicts] = useState<Conflict[]>([]); | ||
| const [pendingSettings, setPendingSettings] = useState<configuration | null>(null); |
There was a problem hiding this comment.
Preserve imported state:* entries through conflict resolution.
When the import hits the conflict dialog, only pendingSettings survives. The imported feature-state payload is discarded, and onResolve writes config keys only, so any state:* entries in that file are silently lost on the conflict path.
💡 Minimal fix sketch
const [conflicts, setConflicts] = useState<Conflict[]>([]);
const [pendingSettings, setPendingSettings] = useState<configuration | null>(null);
+const [pendingState, setPendingState] = useState<Partial<FeatureState> | null>(null);
...
-const { conflicts: detectedConflicts, resolved } = detectConflicts(castSettings);
+const stateKeys = metadataRegistry.getAll().map((feature) => `state:${feature.id}` as const);
+const filteredState = Object.fromEntries(
+ Object.entries(stateEntries).filter(([key]) => stateKeys.includes(key as string))
+) as Partial<FeatureState>;
+
+const { conflicts: detectedConflicts, resolved } = detectConflicts(castSettings);
if (detectedConflicts.length > 0) {
setPendingSettings(resolved);
+ setPendingState(filteredState);
setConflicts(detectedConflicts);
return;
}
...
-const stateKeys = metadataRegistry.getAll().map((feature) => `state:${feature.id}` as const);
-const filteredState = Object.fromEntries(Object.entries(stateEntries).filter(([key]) => stateKeys.includes(key as string)));
await browser.storage.local.set({ ...filteredConfig, ...filteredState });
...
onCancel={() => {
setConflicts([]);
setPendingSettings(null);
+ setPendingState(null);
}}
onResolve={(resolvedConfig) => {
void (async () => {
const validKeys = new Set(Object.keys(defaultConfiguration));
const filteredConfig = Object.fromEntries(Object.entries(resolvedConfig).filter(([key]) => validKeys.has(key)));
- await browser.storage.local.set(filteredConfig);
+ await browser.storage.local.set({ ...filteredConfig, ...(pendingState ?? {}) });
await refreshSettings();
setConflicts([]);
setPendingSettings(null);
+ setPendingState(null);
})();
}}Also applies to: 79-90, 188-205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Settings/components/ImportExportSection.tsx` around lines 35 -
36, The import flow drops imported "state:*" entries when a conflict opens
because only pendingSettings is preserved; update the conflict handling to also
preserve the imported feature-state payload and apply it on resolution:
introduce or reuse a state-holding variable (e.g., pendingState or extend
pendingSettings to include a stateMap) when building conflicts, ensure the
conflict dialog keeps that payload (alongside conflicts and pendingSettings),
and change onResolve (and the conflict resolution merge logic referenced around
the existing conflicts, pendingSettings and onResolve handlers) to write both
configuration keys and any preserved "state:*" entries back into storage instead
of discarding them.
| <div | ||
| className="fixed inset-0 z-50 flex items-start justify-center overflow-y-auto bg-black/60 px-4 py-8 backdrop-blur-sm" | ||
| style={{ | ||
| opacity: visible ? 1 : 0, | ||
| transition: "opacity 200ms ease" | ||
| }} | ||
| > | ||
| <div | ||
| className="w-full max-w-sm rounded-2xl bg-white shadow-2xl dark:bg-[#1e2021]" | ||
| style={{ | ||
| opacity: visible ? 1 : 0, | ||
| transform: visible ? "scale(1)" : "scale(0.92)", | ||
| transition: "transform 250ms cubic-bezier(0.34,1.56,0.64,1), opacity 250ms ease" | ||
| }} | ||
| > | ||
| <div className="px-6 pb-5 pt-6"> |
There was a problem hiding this comment.
Add dialog semantics to the modal container.
The modal should expose role="dialog" and aria-modal="true" plus label/description bindings so screen readers treat it as a proper dialog.
Proposed fix
- <div
+ <div
+ aria-describedby="welcome-modal-description"
+ aria-labelledby="welcome-modal-title"
+ aria-modal="true"
className="fixed inset-0 z-50 flex items-start justify-center overflow-y-auto bg-black/60 px-4 py-8 backdrop-blur-sm"
+ role="dialog"
@@
- <h2 className="text-lg font-bold text-gray-900 dark:text-white">Welcome to YouTube Enhancer!</h2>
+ <h2 className="text-lg font-bold text-gray-900 dark:text-white" id="welcome-modal-title">Welcome to YouTube Enhancer!</h2>
@@
- <p className="mb-5 text-sm text-gray-500 dark:text-gray-400">
+ <p className="mb-5 text-sm text-gray-500 dark:text-gray-400" id="welcome-modal-description">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Settings/components/WelcomeModal.tsx` around lines 75 - 90,
The modal container in WelcomeModal.tsx must be given proper dialog semantics:
on the inner modal wrapper div (the element with className "w-full max-w-sm
rounded-2xl...") add role="dialog" and aria-modal="true", and wire
aria-labelledby and aria-describedby to the IDs of the title and description
elements inside the component (create stable id attributes on the heading and
descriptive text if they don't exist). Ensure the outer overlay remains as-is
for visual backdrop but the accessible label/description IDs point to the
visible title/paragraph so screen readers treat WelcomeModal as a proper dialog.
| const tabs = [ | ||
| { icon: <MdTune size={16} />, id: "basic" as TabId, label: "Basic" }, | ||
| { icon: <MdBuild size={16} />, id: "advanced" as TabId, label: "Advanced" }, | ||
| { icon: <MdPalette size={16} />, id: "appearance" as TabId, label: "Appearance" } | ||
| ]; |
There was a problem hiding this comment.
Localize sidebar/tab text instead of hardcoding English.
Tab labels and sidebar copy are currently hardcoded, which causes mixed-language UI in localized builds.
Also applies to: 192-193
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Settings/Settings.tsx` around lines 164 - 168, The tab labels
are hardcoded in the tabs array (variable tabs with entries using TabId "basic",
"advanced", "appearance"); replace the literal English strings ("Basic",
"Advanced", "Appearance") with localized strings via the app's i18n helper
(e.g., t('settings.basic')) or the existing translation function used elsewhere,
and do the same for any other hardcoded sidebar text referenced around the
Settings component (notably the texts at the other mentioned locations near the
component around lines 192-193) so all labels use the translation keys instead
of plain English.
| const url = chrome.runtime.getURL("src/pages/options/index.html"); | ||
| void chrome.tabs.create({ url }); |
There was a problem hiding this comment.
Mixing chrome and browser APIs.
The file imports browser from webextension-polyfill (line 4) but uses the chrome API directly here and at line 68. This inconsistency can cause issues in non-Chrome browsers. Use browser.runtime.getURL() and browser.tabs.create() instead.
🔧 Proposed fix
function openSettings() {
- const url = chrome.runtime.getURL("src/pages/options/index.html");
- void chrome.tabs.create({ url });
+ const url = browser.runtime.getURL("src/pages/options/index.html");
+ void browser.tabs.create({ url });
window.close();
}And at line 68:
- <p className="text-[10px] text-gray-400">v{chrome.runtime.getManifest().version}</p>
+ <p className="text-[10px] text-gray-400">v{browser.runtime.getManifest().version}</p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const url = chrome.runtime.getURL("src/pages/options/index.html"); | |
| void chrome.tabs.create({ url }); | |
| const url = browser.runtime.getURL("src/pages/options/index.html"); | |
| void browser.tabs.create({ url }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/popup/PopupView.tsx` around lines 54 - 55, In PopupView.tsx replace
direct chrome API calls with the webextension-polyfill API: change uses of
chrome.runtime.getURL and chrome.tabs.create to browser.runtime.getURL and
browser.tabs.create (also update the second occurrence around the code at line
68); ensure you import/use the existing browser symbol from
webextension-polyfill and keep the call signatures the same.
| export function applyTheme(theme: UiTheme, element: HTMLElement): void { | ||
| if (theme.preset === "system") { | ||
| element.removeAttribute("data-theme"); | ||
| } else { | ||
| element.setAttribute("data-theme", theme.preset); | ||
| } | ||
| element.style.setProperty("--accent", theme.accentColor); | ||
| const hsl = hexToHsl(theme.accentColor); | ||
| if (hsl) { | ||
| element.style.setProperty("--accent-dark", `hsl(${hsl.h}, ${hsl.s}%, ${Math.max(hsl.l - 10, 0)}%)`); | ||
| } | ||
| } |
There was a problem hiding this comment.
Theme preset accent colors are overridden by inline styles, breaking theme cohesion.
The CSS defines theme-specific accent colors (purple theme uses #a855f7, ocean theme uses #38bdf8), but applyTheme always sets --accent via inline style (line 23), which has higher specificity than the CSS [data-theme] selectors. When a user selects the purple theme preset with the default accent color from DEFAULT_THEME (#2979ff), they'll see the purple background and text colors but a blue accent instead of the intended purple accent.
This creates visual inconsistency where theme presets don't display their designed color schemes.
Recommended fixes:
- Map preset to default accent: Create a preset-to-accent mapping and update
DEFAULT_THEMEor the theme application logic to use theme-specific defaults - Conditional override: Only call
setPropertyfor--accentwhenaccentColordiffers from the theme's CSS default - Remove CSS accent definitions: If custom accent always overrides, remove the unused CSS
--accentdeclarations from theme blocks
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/uiTheme.ts` around lines 17 - 28, applyTheme currently always
writes --accent inline causing preset CSS values to be overridden; update
applyTheme (and related DEFAULT_THEME/UiTheme handling) to consult a
preset->accent default map (e.g., presetAccentMap for "purple","ocean", etc.)
and only call element.style.setProperty("--accent", ...) when theme.accentColor
is present and differs from the mapped preset default for theme.preset (for
"system" preserve behavior), otherwise remove the inline --accent so the
[data-theme] CSS defaults take effect; keep using hexToHsl(theme.accentColor) to
set --accent-dark only when you actually set --accent.
2 participants
Summary by CodeRabbit
New Features
Documentation