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 brave notification description and textual button color in dark mode #27004

Closed
Pavneet-Sing opened this issue Nov 28, 2022 · 6 comments · Fixed by brave/brave-core#16128
Closed
Assignees
Labels
bug front-end-change This task is a front end task and doesn't need any C++ changes OS/Android Fixes related to Android browser functionality QA/No release-notes/exclude

Comments

@Pavneet-Sing
Copy link

Description

Notification dialog description color is hard to read in dark mode.

  • For description in dark mode: can use #84889C color like wallet
  • For textual button, can use #A0A5EB for dark and #4C54D2 for light like wallet

Note: Make sure to confirm the exact colors with figma if specs are available for dark mode/feature.

Actual result

Screenshot from 2022-11-28 15-08-35

Expected result

Update colors as per theme/figma.

cc: @sujitacharya2005 @deeppandya

@Pavneet-Sing Pavneet-Sing added bug QA/Yes release-notes/include OS/Android Fixes related to Android browser functionality labels Nov 28, 2022
@Pavneet-Sing Pavneet-Sing added the front-end-change This task is a front end task and doesn't need any C++ changes label Nov 28, 2022
@deeppandya
Copy link
Contributor

Thank you @Pavneet-Sing. it seems we are missing dark mode spec here : https://www.figma.com/file/sdVBBufbl2A11hAgPNfyqG/Android-13-Notifications?node-id=366%3A30681&t=kj66ZSk8TrqEpCmt-0
@sujitacharya2005 can you update the issue based on the suggestions ?

@anatolydsgn
Copy link

@deeppandya @timchilds I've just updated the design for this notification screen.

https://www.figma.com/file/sdVBBufbl2A11hAgPNfyqG/Android-13-Notifications?node-id=1243%3A31639&t=7Pd5NQyY0I0HjTn0-1

@deeppandya
Copy link
Contributor

we will be updating night mode colors for this issue.

@anatolydsgn
Copy link

@deeppandya just in case, here's a quick fix without changing the layout. you could just use updated colors and illustration. https://www.figma.com/file/sdVBBufbl2A11hAgPNfyqG/Android-13-Notifications?node-id=363%3A30672&t=JBalKKlQzJNGa6zV-1

@kjozwiak
Copy link
Member

The above requires 1.47.168 or higher for 1.47.x verification. @Uni-verse leave this one for me 👍 I think we'll need a follow up as it still looks like it's not using the correct colours. Looks the same as before. I wouldn't consider this a blocker but we'll need a follow up which I'll create tomorrow AM. Example:

Screenshot_20230111-031302

CCing @deeppandya @sujitacharya2005 @timchilds

@Uni-verse Uni-verse added QA/In-Progress Indicates that QA is currently in progress for that particular issue and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Jan 11, 2023
@kjozwiak
Copy link
Member

Labelling this as QA/No and release-notes/exclude as this actually never made it into 1.47.x or Brave in general. Looks like the fix was never merged as per https://github.com/brave/brave-core/pull/16128/files (don't see any colour/theme fixes). Created #27767 as a follow up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug front-end-change This task is a front end task and doesn't need any C++ changes OS/Android Fixes related to Android browser functionality QA/No release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants