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

AaaLD - article content interstitial design polish #3744

Merged
merged 4 commits into from Oct 27, 2020
Merged

AaaLD - article content interstitial design polish #3744

merged 4 commits into from Oct 27, 2020

Conversation

tonisevener
Copy link
Collaborator

@tonisevener tonisevener commented Oct 23, 2020

Fixes Phabricator ticket:
https://phabricator.wikimedia.org/T265666
https://phabricator.wikimedia.org/T265669

Notes

  • Open up article you've never seen on EN Wikipedia in LTR mode on device.
  • Look at timestamp next to the New Changes badge.
  • Now pull to refresh, confirm new changes badge switches to Last Updated and timestamp looks the same (it was different in some cases before)
  • Confirm you don't see any bugs on the article interstitial (colors and spacing were tweaked). Test all themes.
  • Button background colors on View Changes / View Discussion / Thank buttons in modal slightly changed, confirm that looks alright (note Carolyn still wants to rethink the Thank selected state).

@tonisevener tonisevener requested review from a team and mcleinman and removed request for a team October 23, 2020 19:58
Copy link
Contributor

@mcleinman mcleinman left a comment

Choose a reason for hiding this comment

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

Great tweaks overall. A few questions/comments.

When playing with text sizes, I found another bug. Feel free to fix it here, or spin it out into its own Phab ticket (and assign to me if you want - it's likely a bug I didn't find before, almost definitely not something you introduced).

  • The AaaLD modal uses the device's text size. At the largest (non-accessibility) text size and the smallest text size, open the AaaLD modal. (I didn't check on large accessibility sizes.)
  • Note that the scrollview is scrolled all the way to the bottom - instead of the top, as expected. Also, the navbar underbar view is showing.
  • I'm unclear if this is a bug in the navbar, the auto-scrolling (which shouldn't be happening in this situation), or something else. But I'd be happy to dig in if you'd like.

Wikipedia/Code/ArticleWebMessagingController.swift Outdated Show resolved Hide resolved
www/significant-events-styles-sepia.css Show resolved Hide resolved
@mcleinman mcleinman self-requested a review October 27, 2020 20:06
Copy link
Contributor

@mcleinman mcleinman left a comment

Choose a reason for hiding this comment

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

Working great, thanks!

@mcleinman mcleinman merged commit da88cc2 into main Oct 27, 2020
@mcleinman mcleinman deleted the T265666 branch October 27, 2020 20:07
@mcleinman
Copy link
Contributor

Great tweaks overall. A few questions/comments.

When playing with text sizes, I found another bug. Feel free to fix it here, or spin it out into its own Phab ticket (and assign to me if you want - it's likely a bug I didn't find before, almost definitely not something you introduced).

* The AaaLD modal uses the device's text size. At the largest (non-accessibility) text size and the smallest text size, open the AaaLD modal. (I didn't check on large accessibility sizes.)

* Note that the scrollview is scrolled all the way to the bottom - instead of the top, as expected. Also, the navbar underbar view is showing.

* I'm unclear if this is a bug in the navbar, the auto-scrolling (which shouldn't be happening in this situation), or something else. But I'd be happy to dig in if you'd like.

Phab ticket for this bug: https://phabricator.wikimedia.org/T266621

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