feat(request-button): show input to customize request button label when is4k active#2520
feat(request-button): show input to customize request button label when is4k active#2520s1cstrings wants to merge 6 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 an optional per-server Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "RequestButton UI"
participant API as "Backend API"
participant DB as "Settings Store"
UI->>API: GET /radarr or /sonarr (request servers)
API->>DB: Read Radarr/Sonarr settings (includes requestLabel)
DB-->>API: Return settings with requestLabel
API-->>UI: Respond with server list including requestLabel
UI->>UI: compute button text (base localized text + optional requestLabel)
UI-->>User: Render request button with computed label
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/i18n/locale/uk.json`:
- Around line 569-570: The placeholder strings use a Latin "K" but the Ukrainian
locale uses the Cyrillic "К"; update the JSON values for the keys
components.Settings.RadarrModal.requestButtonLabelPlaceholder and any other
occurrences (e.g., the instances around lines referenced as 717-718) to replace
"4K" with "4К" (Cyrillic K) so typography is consistent across the locale.
My bad. Fixed. |
s1cstrings
force-pushed
the
feature/request-button-label
branch
from
3a0ac6b to
efc1b34
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/Settings/SonarrModal/index.tsx (1)
326-329: Consider simplifying the visibility logic.The condition for
showRequestLabelFieldis somewhat complex. The intent appears to be: show the label field for default servers when this server is 4K, OR when another 4K server exists (so users can distinguish request buttons).The use of
!sonarr?.is4kmeans:
- For new servers (
sonarris null): always evaluates totrue- For existing servers:
trueonly if it was originally non-4KThis prevents showing the field when converting a 4K server to non-4K, which may or may not be the intended behavior. Consider adding a brief comment explaining the logic for future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SonarrModal/index.tsx` around lines 326 - 329, The visibility expression for showRequestLabelField is confusing and has edge cases when sonarr is null vs existing (values.isDefault && (values.is4k || (settings.currentSettings.series4kEnabled && !sonarr?.is4k))); simplify and clarify: explicitly evaluate the three intentions—server is default, server is 4K (values.is4k), or another 4K server exists (settings.currentSettings.series4kEnabled) regardless of whether this is a new or existing server—by rewriting the boolean to reflect that intent and add a brief inline comment above showRequestLabelField explaining the chosen logic and why we check sonarr?.is4k (or why we intentionally ignore it for new servers), referencing the symbols showRequestLabelField, values.isDefault, values.is4k, settings.currentSettings.series4kEnabled, and sonarr?.is4k so future maintainers understand the decision.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/Settings/SonarrModal/index.tsx`:
- Around line 326-329: The visibility expression for showRequestLabelField is
confusing and has edge cases when sonarr is null vs existing (values.isDefault
&& (values.is4k || (settings.currentSettings.series4kEnabled &&
!sonarr?.is4k))); simplify and clarify: explicitly evaluate the three
intentions—server is default, server is 4K (values.is4k), or another 4K server
exists (settings.currentSettings.series4kEnabled) regardless of whether this is
a new or existing server—by rewriting the boolean to reflect that intent and add
a brief inline comment above showRequestLabelField explaining the chosen logic
and why we check sonarr?.is4k (or why we intentionally ignore it for new
servers), referencing the symbols showRequestLabelField, values.isDefault,
values.is4k, settings.currentSettings.series4kEnabled, and sonarr?.is4k so
future maintainers understand the decision.
efc1b34 to
957f736
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/Settings/RadarrModal/index.tsx (2)
293-296: Consider usingvalues.is4kconsistently for the second condition.The current logic uses
radarr?.is4k(original prop) for the second condition instead ofvalues.is4k(current form value). This means the visibility condition partly depends on the server's original 4K state rather than the user's current selection.If user toggles
is4koff after it was on, the field visibility may not update as expected whenmovie4kEnabledis true. This appears intentional (to allow labeling non-4K servers when 4K exists), but the mixed usage of original vs. form values could cause subtle UX inconsistencies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/RadarrModal/index.tsx` around lines 293 - 296, The showRequestLabelField visibility mixes form state and original prop causing inconsistent UI; change the second condition to use the form value (values.is4k) instead of radarr?.is4k so the expression becomes: values.isDefault && (values.is4k || (settings.currentSettings.movie4kEnabled && values.is4k ? ...)) — i.e., replace radarr?.is4k with values.is4k in the showRequestLabelField computation (refer to showRequestLabelField, values.isDefault, values.is4k, settings.currentSettings.movie4kEnabled, and radarr?.is4k) so the field visibility reacts solely to current form values.
249-251: Minor: Submission logic may preserve requestLabel when the field is hidden.The submission includes
requestLabelwhenisDefaultis true, butshowRequestLabelFieldhas additional conditions (values.is4k || movie4kEnabled && !radarr?.is4k). If 4K is later disabled globally, the stored label persists but becomes invisible and uneditable.Consider aligning the submission condition with the display condition:
🔧 Suggested alignment
- requestLabel: values.isDefault - ? values.requestLabel?.trim() || undefined - : undefined, + requestLabel: showRequestLabelField + ? values.requestLabel?.trim() || undefined + : undefined,Note: This would require moving
showRequestLabelFieldcomputation before theonSubmithandler or making it a function accessible within both contexts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/RadarrModal/index.tsx` around lines 249 - 251, The submission currently includes requestLabel whenever values.isDefault is true, which can persist a label even when the field is hidden by the showRequestLabelField logic; update the onSubmit logic to use the same visibility predicate as the UI (the showRequestLabelField conditions based on values.is4k, movie4kEnabled and radarr?.is4k) rather than only values.isDefault so requestLabel is only submitted when the field is actually visible/editable; compute showRequestLabelField (or extract it into a helper function referenced by both the render and the onSubmit code) and use that helper inside the submission to decide whether to include requestLabel.src/i18n/locale/en.json (1)
142-142: Unrelated changes included in this file.These changes appear unrelated to the request button label feature:
- Line 142:
components.DiscoverTvUpcoming.upcomingtv- Line 1269:
components.Setup.signinchanged to "Sign in to your account"If these are intentional as part of this PR, no action needed. Otherwise, consider separating them into a dedicated PR for cleaner change tracking.
Also applies to: 1269-1269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/locale/en.json` at line 142, The PR includes unrelated i18n key changes—specifically "components.DiscoverTvUpcoming.upcomingtv" and "components.Setup.signin"—that should not be mixed with the request button label feature; either revert these two keys in the current branch (undo changes to those JSON entries) or move them into a separate commit/PR so this change only contains the request button label work, ensuring "components.DiscoverTvUpcoming.upcomingtv" and "components.Setup.signin" are not modified here.
🤖 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/Settings/SonarrModal/index.tsx`:
- Around line 326-329: The showRequestLabelField calculation uses the initial
server flag sonarr?.is4k instead of the current form state so the label hides
when toggling 4K off; update the expression in the component where
showRequestLabelField is defined (the line referencing showRequestLabelField,
values.isDefault, values.is4k, settings.currentSettings.series4kEnabled,
sonarr?.is4k) to use values.is4k in place of sonarr?.is4k so the visibility
reflects the current form state while preserving the rest of the condition.
---
Nitpick comments:
In `@src/components/Settings/RadarrModal/index.tsx`:
- Around line 293-296: The showRequestLabelField visibility mixes form state and
original prop causing inconsistent UI; change the second condition to use the
form value (values.is4k) instead of radarr?.is4k so the expression becomes:
values.isDefault && (values.is4k || (settings.currentSettings.movie4kEnabled &&
values.is4k ? ...)) — i.e., replace radarr?.is4k with values.is4k in the
showRequestLabelField computation (refer to showRequestLabelField,
values.isDefault, values.is4k, settings.currentSettings.movie4kEnabled, and
radarr?.is4k) so the field visibility reacts solely to current form values.
- Around line 249-251: The submission currently includes requestLabel whenever
values.isDefault is true, which can persist a label even when the field is
hidden by the showRequestLabelField logic; update the onSubmit logic to use the
same visibility predicate as the UI (the showRequestLabelField conditions based
on values.is4k, movie4kEnabled and radarr?.is4k) rather than only
values.isDefault so requestLabel is only submitted when the field is actually
visible/editable; compute showRequestLabelField (or extract it into a helper
function referenced by both the render and the onSubmit code) and use that
helper inside the submission to decide whether to include requestLabel.
In `@src/i18n/locale/en.json`:
- Line 142: The PR includes unrelated i18n key changes—specifically
"components.DiscoverTvUpcoming.upcomingtv" and "components.Setup.signin"—that
should not be mixed with the request button label feature; either revert these
two keys in the current branch (undo changes to those JSON entries) or move them
into a separate commit/PR so this change only contains the request button label
work, ensuring "components.DiscoverTvUpcoming.upcomingtv" and
"components.Setup.signin" are not modified here.
957f736 to
6773ac6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/Settings/RadarrModal/index.tsx`:
- Around line 293-296: The visibility condition for showRequestLabelField is too
strict because using !radarr?.is4k hides the field when editing an existing 4K
server that the user unchecks; update the condition in the component to remove
the radarr?.is4k check so the field shows whenever values.isDefault is true and
either values.is4k or settings.currentSettings.movie4kEnabled is true (i.e.,
change the showRequestLabelField logic used in this file to use values.isDefault
&& (values.is4k || settings.currentSettings.movie4kEnabled)); locate the
identifier showRequestLabelField and adjust the boolean expression accordingly.
| const showRequestLabelField = | ||
| values.isDefault && | ||
| (values.is4k || | ||
| (settings.currentSettings.movie4kEnabled && !radarr?.is4k)); |
There was a problem hiding this comment.
Minor UX inconsistency in field visibility logic.
The condition !radarr?.is4k causes the field to disappear when editing an existing 4K server and unchecking the "4K Server" checkbox, even if movie4kEnabled is true. This is inconsistent with creating a new non-4K server where the field would be visible.
Consider simplifying to show the field whenever the label would be relevant:
Suggested fix
const showRequestLabelField =
values.isDefault &&
- (values.is4k ||
- (settings.currentSettings.movie4kEnabled && !radarr?.is4k));
+ (values.is4k || settings.currentSettings.movie4kEnabled);📝 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 showRequestLabelField = | |
| values.isDefault && | |
| (values.is4k || | |
| (settings.currentSettings.movie4kEnabled && !radarr?.is4k)); | |
| const showRequestLabelField = | |
| values.isDefault && | |
| (values.is4k || settings.currentSettings.movie4kEnabled); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Settings/RadarrModal/index.tsx` around lines 293 - 296, The
visibility condition for showRequestLabelField is too strict because using
!radarr?.is4k hides the field when editing an existing 4K server that the user
unchecks; update the condition in the component to remove the radarr?.is4k check
so the field shows whenever values.isDefault is true and either values.is4k or
settings.currentSettings.movie4kEnabled is true (i.e., change the
showRequestLabelField logic used in this file to use values.isDefault &&
(values.is4k || settings.currentSettings.movie4kEnabled)); locate the identifier
showRequestLabelField and adjust the boolean expression accordingly.
|
Running |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🤖 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/Settings/SonarrModal/index.tsx`:
- Around line 92-103: The helper shouldShowRequestLabelField is using the
original sonarrIs4k flag causing visibility to not follow the form toggle;
update the logic in shouldShowRequestLabelField to rely on the current is4k
value (or simplify to return series4kEnabled || is4k) instead of sonarrIs4k so
the field reflects the live toggle, and apply the same change to the other
duplicate helper usages (the duplicates around the other occurrences noted) so
all visibility checks use the current is4k/form state.
Yeah, that's not the first time I see this, don't worry. |
Ahhh thanks for the info. I was going crazy trying to figure out why i18n is failing. If there's anything I need to do to on my end to get this resolved just let me know. |
Its been something that has been happening intermittently and we haven't really checked it out yet since busy and it's mostly harmless 😅 |
s1cstrings
force-pushed
the
feature/request-button-label
branch
from
34f3d07 to
7ef9a7c
Compare
…en is4k active When a 4k server is enabled, an input appears below the 4k checkbox to let people edit the request button labels. Both default server modal have their own input. If left empty, nothing is changed and default labels are used. If populated, the text is appended after localized "Request" or "Request More". This is an optional visual UI change only and the behavior of is4k is unchanged. Great for people using the 4k server for other uses cases.
fixed ukrainian translation as per pipeline coderabbit suggestion
removed all translations i had previously added to translation files and ran pnpm i18n:extract as instructed
align requestLabel submission with field visibility in Radarr/Sonarr modals, addressing valid nitpick from coderabbit.
s1cstrings
force-pushed
the
feature/request-button-label
branch
from
7ef9a7c to
98d442f
Compare
|
i rebased after a week+ and now the unrelated entries are somehow magically gone. |
3 participants
Description
When a 4k server is enabled, an input appears below the 4k checkbox to let people edit the request button labels. Both default server modal have their own input. If left empty, nothing is changed and default labels are used. If populated, the text is appended after localized "Request" or "Request More". This is an optional visual UI change only and the behavior of is4k is unchanged. Great for people using the 4k server for other uses cases.
AI Assistance Notice
I used codex 5.3 to generate the first draft, but verified and heavily modified manually.
How Has This Been Tested?
-Ran locally with both pnpm and docker connected to a test environment of jellyfin, 3 radarr and 3 sonarr.
-Currently running in my production setup with actual data and users.
-Tested all edge cases i could possibly think of.
Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
Documentation
UI Updates