Skip to content

Conversation

@subsymbolic
Copy link
Contributor

@subsymbolic subsymbolic commented Oct 8, 2018

Task/Issue URL: https://app.asana.com/0/361428290920652/832439960662903

Description:
Light theme polish

Steps to test this PR:
NOTE: it may be helpful to comment out the sa, sb and mg variants in VariantManager.kt.

Light theme tweaks:

  1. Go to settings
  2. Switch to light theme
  3. Note the gray background of the toggles show
  4. Press the up button, note the ripple/shadow shows
  5. Visit the tabs switcher screen, note the lighter background.
  6. Note also that the unselected tabs are no longer grayed out (this also applies in dark mode)

Ensure omnibar does not disappear

  1. Open the settings and switch between light and dark modes a few times
  2. Press the back button to go back to the browser screen
  3. Ensure the omnibar is still visible

Internal references:

Software Engineering Expectations
Technical Design Template

@subsymbolic subsymbolic requested a review from brindy October 8, 2018 22:35
Copy link
Contributor

@brindy brindy left a comment

Choose a reason for hiding this comment

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

Looks good - the status bar is dark when switching from light to dark, but that seems to only be problem on a small number of emulators/devices.

@brindy
Copy link
Contributor

brindy commented Oct 9, 2018

Hold off merging - the overflow menu on the bookmarks screen is black text on dark background. Tried it on an API 23 and API 27 emulator...

@subsymbolic
Copy link
Contributor Author

Good catch, I have updated the overflow menu color. Thanks for the review @brindy.

Copy link
Contributor

@brindy brindy left a comment

Choose a reason for hiding this comment

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

LGTM!

@subsymbolic subsymbolic merged commit 2d4a309 into develop Oct 9, 2018
@subsymbolic subsymbolic deleted the feature/theming_polish branch October 9, 2018 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants