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

PLAYRTS-5590 Update Highlight Cell Design #504

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mutaben
Copy link
Contributor

@mutaben mutaben commented Jul 18, 2024

Description

Up until now, there was a linear gradient on Highlight Cells, helping make the text more legible.

This PR improves this aspect based on UI/UX requirements, by improving legibility without darkening the highlighted content too much.

In the following images, you can see the raw gradients, how they used to be and how they were implemented after various attempts to find the best result.

Please see also some screenshots to see it in action today.




Changes Made

This was implemented by:

  • Moving the description text at the bottom left as requested
  • Tweaking the gradients, two gradients are now used (both linear):
    • One from bottom to top, in order to highlight the description text
    • One from the bottom left corner

The gradients were tweaked for large and compact size, and for different amount of text content. (see the raw gradients above)

If they need to be adjusted in the future, they now have their own view and the midway point to stop the gradient is customizable as a parameter.

Checklist

  • I have followed the project's style guidelines.
  • I have performed a self-review of my own changes.
  • I have made corresponding changes to the documentation.
  • My changes do not generate new warnings.
  • I have tested my changes and I am confident that it works as expected and doesn't introduce any known regressions.
  • I have reviewed the contribution guidelines.

@mutaben mutaben added the improvement Feature or update (issue and PR) - release notes section label Jul 18, 2024
@mutaben mutaben requested a review from pyby July 18, 2024 13:46
@pyby pyby changed the title PLAYRTS-5590 - Update Highlight Cell Design PLAYRTS-5590 Update Highlight Cell Design Jul 22, 2024
@pradie-charlotte
Copy link

Did you also change highlight's dimensions ? ( mobile 200, tablet portrait 280, and tablet landscape 480)
Cause on android tablet (landscape mode), highlight looks a little bit large. Have you got the same impression/ result on ios ?
You can see the android tablet landscape result on the android merge request

@mutaben
Copy link
Contributor Author

mutaben commented Jul 26, 2024

Did you also change highlight's dimensions ? ( mobile 200, tablet portrait 280, and tablet landscape 480) Cause on android tablet (landscape mode), highlight looks a little bit large. Have you got the same impression/ result on ios ? You can see the android tablet landscape result on the android merge request

@pradie-charlotte
No, we defined only two highlights, one for compact, and one for regular, and I tried to stick to the Figma. Hope that helps.

@pyby
Copy link
Member

pyby commented Jul 29, 2024

@pradie-charlotte No, we defined only two highlights, one for compact, and one for regular, and I tried to stick to the Figma. Hope that helps.

@mutaben But if the UX team proposes a third one compatible with Apple rotation and size classes, this iPad may could do this:
ShowHeaderView (header) and HeroMediaCell (content cell, like the highlighted one) have three layouts:

  • horizontal compact.
  • horizontal regular in portrait.
  • horizontal regular in landscape.

If the Android test on tablet is validated, let check on iPad then.

@rts-devops rts-devops temporarily deployed to playsrg-ios-nightly+PLAYRTS-5590-highlight-change July 29, 2024 14:55 Inactive
@rts-devops rts-devops temporarily deployed to playsrg-tvos-nightly+PLAYRTS-5590-highlight-change July 29, 2024 14:55 Inactive
@Loic-Dumas
Copy link
Contributor

Loic-Dumas commented Jul 30, 2024

Hello @mutaben @pyby ,
To clarify the current status on Android.

As explained by @pradie-charlotte, we have these height for the highlight, as defined in the Figma:

  • Mobile 200 (the view is called packed, as there's no description)
  • Tablet portrait 280
  • Tablet landscape 480
  • TV 250 (We kept the 250 , as the 480 was clearly too much)

We Asked Alessandra if for the tablet land 480 was too much. But since she's on holiday, we decided to merge, and update the height later if needed.

Copy link
Member

@pyby pyby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure VerticalGradientView(midwayStop: 0.3) or the VerticalGradientView height is enough for light background images.

Nice stops improvement.

Application/Sources/UI/Views/HighlightCell.swift Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Feature or update (issue and PR) - release notes section
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants