Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui): remove 'all' badge from request cards #2992

Merged
merged 1 commit into from
Sep 16, 2022
Merged

fix(ui): remove 'all' badge from request cards #2992

merged 1 commit into from
Sep 16, 2022

Conversation

aedelbro
Copy link
Contributor

@aedelbro aedelbro commented Aug 28, 2022

Updates both RequestList/RequestItem and RequestCard components to no longer show ALL if all seasons are requested.

Originally, the number of requested seasons was added onto the ALL request (ie ALL - 4). After some discussion, it was decided to remove the ALL badge to keep the requests consistent.

Description

When viewing the request list / request cards, if ALL seasons are requested, it will show each of the seasons requested, compared to the ALL badge that would show before.

Original PR Updates both RequestList/RequestItem and RequestCard components to contain number of seasons if 'ALL' seasons are requested

Description

When viewing the request list / request cards, if ALL seasons are requested, this will include the number of seasons in the badge. If scrolling through the list of requests, the user no longer needs to open the details in a new tab to see how many seasons have been requested

Screenshot (if UI-related)

Request List:

Screen Shot 2022-08-28 at 1 50 46 PM

becomes:

Screen Shot 2022-08-28 at 1 51 17 PM

Request Card:

Screen Shot 2022-08-28 at 1 52 17 PM

becomes:

Screen Shot 2022-08-28 at 1 51 56 PM

To-Dos

  • Successful build yarn build

Note: While this isn't technically a bug, I chose fix over feat, as I didn't think it really qualified as a feature. Also, first time contributing, so hope this PR is up to standards 馃槃

@TheCatLady
Copy link
Collaborator

I think a better alternative would be to simply remove the logic for the ALL badge and display the season list, for consistency. 馃

@aedelbro
Copy link
Contributor Author

Personally, I like having the ALL summary badge. If there's a large number of seasons being requested (for example One Piece has 21), it becomes really hard to see what is being requested (granted having this many seasons is rare).
For the RequestCard, you can't tell if there's more than 5 seasons being requested.
For the RequestItem, (at my laptop screen resolution) you can't tell if there's more than 18 seasons being requested.

While there is a horizontal scroll, there's no indication of that.

My use case (and I'm sure many others), is typically to request all seasons of a series on discovery. Then as a new seasons airs, only that 1 season is requested.

Love to hear your thoughts as well 馃

Comparison shots:

Screen Shot 2022-08-29 at 10 05 04 AM

vs

Screen Shot 2022-08-29 at 10 04 08 AM

and

Screen Shot 2022-08-29 at 10 14 57 AM

vs

Screen Shot 2022-08-29 at 10 13 50 AM

@TheCatLady
Copy link
Collaborator

Personally, I like having the ALL summary badge. If there's a large number of seasons being requested (for example One Piece has 21), it becomes really hard to see what is being requested (granted having this many seasons is rare). For the RequestCard, you can't tell if there's more than 5 seasons being requested. For the RequestItem, (at my laptop screen resolution) you can't tell if there's more than 18 seasons being requested.

While there is a horizontal scroll, there's no indication of that.

This is a UX issue that should be addressed, because it doesn't only apply to series where all seasons have been requested. It could also apply to series like One Piece where all but a few season were requested. I still feel we should be consistent in how we display the requested seasons.

I have never liked the ALL badge, and I feel like the addition of the season count makes it look even more tacky. I think we need to come up with some other to indicate that a series is fully requested.

My use case (and I'm sure many others), is typically to request all seasons of a series on discovery. Then as a new seasons airs, only that 1 season is requested.

If a series is requested (and thus monitored in Sonarr), new seasons will actually automatically be monitored and downloaded (this is not configurable, just how Sonarr works). Additional requests do not need to be submitted in Overseerr to grab new seasons.

@aedelbro
Copy link
Contributor Author

If a series is requested (and thus monitored in Sonarr), new seasons will actually automatically be monitored and downloaded (this is not configurable, just how Sonarr works). Additional requests do not need to be submitted in Overseerr to grab new seasons.

I don't use Sonarr, as I like being picky about choosing sources 馃槄 , so I was unaware of that. Most people probably use Sonarr, so I guess strike that thought.

I have never liked the ALL badge, and I feel like the addition of the season count makes it look even more tacky. I think we need to come up with some other to indicate that a series is fully requested.

I've been mulling it over a bit more, and I can go ahead and remove the ALL badge logic you suggested. Even though I like the extra info of knowing everything has been requested, showing each individual season badge still address my main concern (having to open another tab to see what seasons have been requested).

Then there can be a proper separate discussion about how to indicate that all seasons have been requested (not sure if this PR is the place for that, if it is, I can keep it open).

Additionally, the UX of not being able to tell if there's more seasons to show, that can be addressed in a separate PR 馃槃 (Actual UI changes might be beyond my limited frontend experience, but I might try if I have time).

@TheCatLady
Copy link
Collaborator

If a series is requested (and thus monitored in Sonarr), new seasons will actually automatically be monitored and downloaded (this is not configurable, just how Sonarr works). Additional requests do not need to be submitted in Overseerr to grab new seasons.

I don't use Sonarr, as I like being picky about choosing sources 馃槄 , so I was unaware of that. Most people probably use Sonarr, so I guess strike that thought.

Ah, well if you don't use Sonarr, then those new seasons would not be automatically monitored (and thus Overseerr wouldn't mark them as "Requested" during a scan). So, you would be able to continue using Overseerr as you are currently.

P.S.- I am also super picky about media sources, btw -- Sonarr is highly configurable and you should be able to tweak it to honor your preferences.

@aedelbro aedelbro changed the title fix(ui): add number of seasons requested to 'ALL' requests feat(ui): remove 'all' badge from request cards Sep 13, 2022
@aedelbro
Copy link
Contributor Author

@TheCatLady Sorry for the delay in making the fix; but got the commit and PR updated.

Where should the discussion take place, if at all, for showing if all seasons are requested? Discord / here?

Related but for a separate PR: I had some thoughts on the request card, in showing the season available / partially available / unavailable. A green badge for the season being available, a stripped green / purple for partially available, and the purple for requested.
Quick mock up like this:
Capture

If that sounds like a good idea, it could replace all of the partially available green with the stripped green / purple. I know contrast / accessibility could potentially be a concern with the stripes, so I wanted a full discussion before putting in work on it.

Any additional thoughts? Thanks 馃槃

@TheCatLady
Copy link
Collaborator

Where should the discussion take place, if at all, for showing if all seasons are requested? Discord / here?

The development channel in our Discord server is the best place to discuss.

Related but for a separate PR: I had some thoughts on the request card, in showing the season available / partially available / unavailable. A green badge for the season being available, a stripped green / purple for partially available, and the purple for requested. Quick mock up like this: Capture

I really dislike the stripes... but the idea to show available items a different color is interesting. Let's move this discussion to Discord :)

if all seasons are requested for a TV show, show each indivdual season badge. This prevents the
admin from needing to open a second tab / navigate to see how many seasons / what seasons have been
requested.
@TheCatLady TheCatLady changed the title feat(ui): remove 'all' badge from request cards fix(ui): remove 'all' badge from request cards Sep 16, 2022
@TheCatLady TheCatLady enabled auto-merge (squash) September 16, 2022 21:19
@TheCatLady TheCatLady merged commit 5c01313 into sct:develop Sep 16, 2022
@github-actions
Copy link

馃帀 This PR is included in version 1.30.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants