feat: Improve 4K content visibility and related user experience#3060
feat: Improve 4K content visibility and related user experience#3060Flawkee wants to merge 6 commits into
Conversation
…ser experience improve 4K content visibility and enable inline request from thumbnails + status and availability + separate 4k request button in title page admin config "re seerr-team#2598", "re seerr-team#1826"
next.js changed the import when tested locally - reveted it
…ted, available, etc) larger This commit makes details page badges for request status (requested, available, partially available, etc...) bigger and more visible to avoid user duplicate requests. "re seerr-team#2993"
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds boolean setting Changes4K Separate Request Button and Status Display Feature
Sequence DiagramsequenceDiagram
participant User
participant TitleCard
participant Settings
participant RequestButton
participant RequestModal
participant API
User->>TitleCard: view title (includes status4k)
TitleCard->>Settings: read separate4kRequestButton & permissions
Settings-->>TitleCard: returns flags
TitleCard->>RequestButton: render actions
RequestButton->>RequestButton: split into mainButtons + fourKButtons (if enabled)
User->>RequestButton: click 4K request
RequestButton->>RequestModal: open modal (is4k=true)
User->>RequestModal: confirm
RequestModal->>API: POST request (is4k=true)
API-->>RequestModal: success (response.data.media.status4k)
RequestModal->>TitleCard: onComplete(status4k)
TitleCard->>TitleCard: update currentStatus4k
TitleCard-->>User: show updated 4K badge/state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/TitleCard/index.tsx`:
- Around line 367-369: The 4K action visibility variables (showRequest4k,
showAvailable4k, showRequested4k) should also guard against blocklisted titles;
update each expression that uses show4k &&
isRequestableStatus(currentStatus4k)/isAvailableStatus(currentStatus4k)/isRequestedStatus(currentStatus4k)
to additionally check currentStatus4k !== MediaStatus.BLOCKLISTED (or an
equivalent isBlocklistedStatus check) so blocklisted cards do not render any 4K
action buttons; locate these symbols (showRequest4k, showAvailable4k,
showRequested4k, show4k, currentStatus4k, MediaStatus.BLOCKLISTED) in
TitleCard/index.tsx and add the extra condition to each boolean expression.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f9bd8e0-713f-4c96-9609-71cdb667285f
📒 Files selected for processing (19)
server/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tssrc/components/CollectionDetails/index.tsxsrc/components/Common/Badge/index.tsxsrc/components/Common/ListView/index.tsxsrc/components/Common/StatusBadgeMini/index.tsxsrc/components/MediaSlider/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/PersonDetails/index.tsxsrc/components/RequestButton/index.tsxsrc/components/RequestModal/TvRequestModal.tsxsrc/components/Settings/SettingsMain/index.tsxsrc/components/StatusBadge/index.tsxsrc/components/TitleCard/TmdbTitleCard.tsxsrc/components/TitleCard/index.tsxsrc/components/TvDetails/index.tsxsrc/context/SettingsContext.tsxsrc/i18n/locale/en.jsonsrc/pages/_app.tsx
…tons As code rubbit suggested, I added a blacklist guard for 4K title card buttons - honest mistake
I reverted en.json change by i18n extract by mistake, so I ran it again to pass the CI check
|
We already have an existing implementation for this in Overseerr that I’ve essentially ported over to Seerr: That PR was created by one of our team members, and its design aligns with our current design language. This PR conflicts with that implementation, and there are several design elements here that do not align with Seerr’s existing UI patterns. Before continuing with implementation changes, I’d really appreciate it if you could open a meta discussion in the Discussions tab so the team can align on the intended design direction first: That would help us avoid situations where significant implementation work ends up needing major redesign changes later on, which can be frustrating for both contributors and reviewers. For the time being, could you please convert this PR to a draft while the design discussion takes place? CC: @seerr-team/seerr-core |
|
@fallenbagel This PR is addressing multiple issues. I have created the discussion #3158 |
…ity Requests are allowed A new admin configuration that is enabled by default allowing users to request both normal and 4k quality of a title (current behavior). If disabled, when a title is requested in either quality, the other quality cannot be requested (unless you are admin \ requests manager)
| const requestRepository = getRepository(MediaRequest); | ||
| const removedQualityRequests = media.requests.filter( | ||
| (request) => request.is4k === is4k | ||
| ); | ||
| const remainingRequests = media.requests.filter( | ||
| (request) => request.is4k !== is4k | ||
| ); | ||
|
|
||
| if (removedQualityRequests.length > 0) { | ||
| await requestRepository.remove(removedQualityRequests); | ||
| } | ||
|
|
||
| media.requests = remainingRequests; | ||
|
|
||
| const otherQualityStatus = is4k ? media.status : media.status4k; | ||
| const otherQualityInUse = | ||
| remainingRequests.length > 0 || | ||
| (otherQualityStatus !== MediaStatus.UNKNOWN && | ||
| otherQualityStatus !== MediaStatus.DELETED); | ||
|
|
||
| if (otherQualityInUse) { | ||
| media[is4k ? 'status4k' : 'status'] = MediaStatus.UNKNOWN; | ||
| media[is4k ? 'serviceId4k' : 'serviceId'] = null; | ||
| media[is4k ? 'externalServiceId4k' : 'externalServiceId'] = null; | ||
| media[is4k ? 'externalServiceSlug4k' : 'externalServiceSlug'] = null; | ||
| media[is4k ? 'ratingKey4k' : 'ratingKey'] = null; | ||
| media[is4k ? 'jellyfinMediaId4k' : 'jellyfinMediaId'] = null; | ||
|
|
||
| if (media.mediaType === MediaType.TV) { | ||
| for (const season of media.seasons) { | ||
| season[is4k ? 'status4k' : 'status'] = MediaStatus.UNKNOWN; | ||
| } | ||
| } | ||
|
|
||
| await mediaRepository.save(media); | ||
| } else { | ||
| await mediaRepository.remove(media); | ||
| } | ||
|
|
There was a problem hiding this comment.
I know this PR is being discussed first on its implementation, but, this is out of scope for a 4K visibility PR.
Also, deleting the file from Radarr/Sonarr shouldn't delete the request at all. Removing here should just reset the media status for that quality. Request deletion is what the clear/remove-media endpoint is for, so this logic shouldn't exist here regardless of scope.
There was a problem hiding this comment.
I can revert it in case you guys decide to move on with this PR.
Anyway - this bug fix resolves an issue when admin clicks Remove from Sonarr/Radarr in Seerr, let's say from 4K instance, and the requests and media status is reset for both instances of that title.
That's an actual bug..
There was a problem hiding this comment.
That's an actual bug..
Doesn't matter... Once again, out of scope of this PR entirely.
Deleting requests was never the intended behavior, this is an oversight, and it'll get fixed in another PR. Scoping it to the correct quality doesn't change that. It still has no business being in a PR about 4K visibility.
Split it out. This touches delete/cascade semantics across a frontend component and a route, it needs its own focused review, not to ride in as a side effect of an unrelated feature.
Also, the fix hard deletes the request row. Elsewhere, orphaned requests get declined if in-flight, or just stay completed if already completed, not deleted. So this isn't really a fix for the actual bug, which is that the button should not be touching requests in the first place and that's the oversight to fix.
There was a problem hiding this comment.
@fallenbagel you are right. I reverted the "bug fix" commit and completly discarded it from the working tree.
Now this PR is only about the 4K UX, as intended.
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
17e445e to
886326a
Compare
2 participants
Description
This completes three requests:
Until now, to request a title in 4K you had to open the title page and the 4K request was hidden inside a dropdown under the normal Request button, so a lot of people miss it. On top of that the thumbnail never showed when a title is already available in 4K, only the normal quality. So users were requesting titles again that already exist in 4K and it created duplicates.
In addition, the status badges in TV\Movie details page was small and hard to miss for less-intuitive users.
What I did:
No database migration is needed, the new setting is stored in settings.json and falls back to false for existing installs.
How Has This Been Tested?
Tested manually on desktop and on mobile (the mobile layout needed some specific fixes for the badges).
My environment was HD Radarr + 4K Radarr and HD Sonarr + 4K Sonarr, with Emby connected for the availability sync on localhost.
Then I tested again on a production setup similar to this with thousands of requests and titles.
What I checked:
Screenshots / Logs (if applicable)
Request in 4K now available on the title cards:
When requested, instead of removing the button, preserve it to show current status + link to the details page:
4K have its own badge and status shows up on the card:
TV\Movie detail page status badges are now bigger and harder to miss yet still elegant and fit the theme:
New admin setting in General section to allow separate 4K and regular requests buttons (false by default to preserve current dropdown behavior):
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
UI / Style
Localization