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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui): display "Season" vs. "Seasons" as appropriate, and fix request block "Seasons" formatting #1127

Merged
merged 2 commits into from Mar 10, 2021

Conversation

TheCatLady
Copy link
Collaborator

@TheCatLady TheCatLady commented Mar 8, 2021

Description

Fixes cards to display "Season" if only one season is requested (if "ALL" badge is displayed, still shows "Seasons").

Also fixes formatting on request block; "Seasons" text was not the same color as other text in the request block.

Screenshot (if UI-related)

image

To-Dos

  • Successful build yarn build
  • Translation keys yarn i18n:extract

Issues Fixed or Closed

N/A

@TheCatLady TheCatLady requested a review from sct as a code owner March 8, 2021 23:03
@sct
Copy link
Owner

sct commented Mar 8, 2021

I actually don't like the plurality here. I think it's fine to say Seasons for both a single selection and multiple.

@TheCatLady
Copy link
Collaborator Author

I actually don't like the plurality here. I think it's fine to say Seasons for both a single selection and multiple.

Ah really? I think it's really odd and looks like a mistake. 😕 We display the correct plurality for networks/studios on the detail pages, and I really think we should fix this here as well.

@sct
Copy link
Owner

sct commented Mar 8, 2021

Ah really? I think it's really odd and looks like a mistake. 😕 We display the correct plurality for networks/studios on the detail pages, and I really think we should fix this here as well.

I don't agree. Networks/Studios are a different case. Seasons is a list of badges representing each season. Feels better to me to just have it say one thing in both cases. 🤷‍♂️

@TheCatLady
Copy link
Collaborator Author

TheCatLady commented Mar 8, 2021

I don't agree. Networks/Studios are a different case. Seasons is a list of badges representing each season. Feels better to me to just have it say one thing in both cases. 🤷‍♂️

I think it would be particularly bad in cases where a single season is requested and it isn't the first season. Because then you'd have "Seasons: [2]" for example, and it could be interpreted as a season count rather than a season number.

@TheCatLady TheCatLady force-pushed the fix/ui/season-count-plurality branch 2 times, most recently from a35d280 to 4ff8b14 Compare March 9, 2021 13:10
@TheCatLady TheCatLady force-pushed the fix/ui/season-count-plurality branch from 4ff8b14 to 104584d Compare March 10, 2021 02:33
@sct sct merged commit 45886cc into develop Mar 10, 2021
@sct sct deleted the fix/ui/season-count-plurality branch March 10, 2021 02:39
@github-actions
Copy link

🎉 This PR is included in version 1.21.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