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.

Sign up for GitHub

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

Jump to bottom

Rework / sprint 3 rs #146

Merged
merged 7 commits into from
Feb 28, 2024
Merged

Rework / sprint 3 rs #146

merged 7 commits into from
Feb 28, 2024

Conversation

royschut
Copy link

@royschut royschut commented Feb 28, 2024 •
edited
Loading

Rework Sprint 3 RS

  • Fix new subscription not showing in UI - OTT-992
  • Fix cinema not covering full viewport height - OTT-1022, OTT-1030, OTT-4043
  • Fix page behind player too wide - OTT-1010
  • Fix button alignment video detail mobile (see before/after below)
  • Feat: align sidebar backdrop with modal: much less transparent and a gentle animation added
  • Fix: add selected state to season filters - OTT-1004
  • Add role to make lock icon readable (axetools warned about this, and I've confirmed it now gets read by the screen reader)

Notes

  • New subscription fix: I find Feat / retry reload subscription #147 more safe, but can imagine we find it risky
  • The 'page too wide' issue was the result of the Sidebar scroller fix. I've reverted the negative margin. The downside of this revert is a narrow empty black area on Safari mobile, but I found this way more acceptable than the background next to the Cinema. Also, the new darker backdrop makes it less of an issue.

Screenshot 2024-02-28 at 14 04 06

Screenshot 2024-02-28 at 14 04 00

@royschut royschut marked this pull request as ready for review February 28, 2024 14:02
langemike
langemike approved these changes Feb 28, 2024
View reviewed changes
Copy link

@langemike langemike left a comment

Choose a reason for hiding this comment

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

Nice list of fixes! Don't forget to update the snapshots! I already approve the change
I did a really quick test on the deploy preview

MelissaDTH
MelissaDTH approved these changes Feb 28, 2024
View reviewed changes
Copy link

@MelissaDTH MelissaDTH left a comment

Choose a reason for hiding this comment

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

LGTM!

@royschut
fix(a11y): add role to make lock icon readable
b08159a 
refactor(tests): update snapshots
@royschut royschut merged commit 56aa228 into feat/sprint-3 Feb 28, 2024
7 checks passed
@royschut royschut deleted the rework/sprint-3-rs branch February 28, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MelissaDTH MelissaDTH MelissaDTH approved these changes

@ChristiaanScheermeijer ChristiaanScheermeijer ChristiaanScheermeijer approved these changes

@langemike langemike langemike approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@royschut @langemike @ChristiaanScheermeijer @MelissaDTH