Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] "Share" menu "Recently Used" is not colored in Dark theme #8984

Closed
Cheap-Skate opened this issue Mar 5, 2020 · 10 comments · Fixed by #18839
Closed

[Bug] "Share" menu "Recently Used" is not colored in Dark theme #8984

Cheap-Skate opened this issue Mar 5, 2020 · 10 comments · Fixed by #18839
Assignees
@neha-b2001
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Themes Dark mode, light mode, private browsing mode good first issue Good for newcomers help wanted Help wanted from a contributor. More complex than good first issue. S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist
Projects
Android Engineering Team Kanban board
Milestone
90

Comments

@Cheap-Skate
Copy link

Cheap-Skate commented Mar 5, 2020 •
edited by data-sync-user

Steps to reproduce

In Light theme, the "Share" menu "Recently Used" items list has a shaded roundrect
In Dark theme, there is no shading

Expected

Dark theme share menu should have a rounded rectangle using the same shade defined in light mode

Actual

No shading around Recently used

  • Android device: Galaxy A40
  • Fenix version: Android 9.0.0

┆Issue is synchronized with this Jira Task
@Cheap-Skate Cheap-Skate added the 🐞 bug Crashes, Something isn't working, .. label Mar 5, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Mar 5, 2020
@mcarare
Copy link
Contributor

mcarare commented Mar 5, 2020

@darkwing This seems to be a regression from #8839 like #8949. Can you please check? TY!

@darkwing
Copy link
Contributor

Could you post a screenshot? I'm not understanding what the issue is based on description.

@Cheap-Skate
Copy link
Author

Cheap-Skate commented Mar 16, 2020 •
edited by eliserichards

In Light mode there is a gray roundrect to distinguish "recently used" from "all actions". In Dark mode there is no roundrect.

Also, I think that in Dark mode the Android navigation bar changes color (from very dark gray to black) when I move to the Share menu

Screenshot_20200316-213400_Firefox Nightly Screenshot_20200316-213342_Firefox Nightly

@sflorean sflorean added Feature:Themes Dark mode, light mode, private browsing mode S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist and removed needs:triage Issue needs triage labels Oct 29, 2020
@eliserichards eliserichards added good first issue Good for newcomers help wanted Help wanted from a contributor. More complex than good first issue. Outreachy labels Apr 2, 2021
@eliserichards
Copy link
Contributor

eliserichards commented Apr 2, 2021 •
edited

This menu is here:

fenix/app/src/main/res/layout/share_to_apps.xml

Line 28 in 1f320fd

android:id="@+id/recentAppsContainer"

@drawable/recent_apps_background needs to define a light theme and dark theme

@ZianeA
Copy link
Contributor

ZianeA commented Apr 3, 2021 •
edited

I investigated the problem and it seems to originate from the color resource file. Both inset_dark_theme and above_dark_theme use the same color value, which is not the case for the light versions inset_light_theme and above_light_theme. The solution is to use a slightly lighter color for inset_dark_theme. @eliserichards let me know the color and I'll fix it.

Dark:

fenix/app/src/main/res/values/colors.xml

Line 93 in 1f320fd

<color name="inset_dark_theme">#32313C</color>

Light:

fenix/app/src/main/res/values/colors.xml

Line 21 in 1f320fd

<color name="inset_light_theme">#E0E0E6</color>

@eliserichards eliserichards added the needs:UX-feedback Needs UX Feedback label Apr 5, 2021
@eliserichards
Copy link
Contributor

We would like to use #52525E for inset_dark_theme (confirmed with @topotropic).

Are you an Outreachy applicant @ZianeA? If so, please feel free to pick this up 👍

@eliserichards eliserichards removed the needs:UX-feedback Needs UX Feedback label Apr 6, 2021
@ZianeA
Copy link
Contributor

ZianeA commented Apr 6, 2021

Sorry for not noticing that this was for Outreachy applicants. I'm new here, this is my first time contributing to the project. Still, I hope my input was helpful.

@eliserichards
Copy link
Contributor

No worries! Your input was extremely helpful! Sorry about that 😅

@neha-b2001
Copy link
Collaborator

@eliserichards Could I please work on this issue (I'm an Outreachy applicant)?

@neha-b2001 neha-b2001 mentioned this issue Apr 7, 2021
Merged
3 tasks
@gabrielluong gabrielluong added this to the 89 milestone Apr 8, 2021
@gabrielluong gabrielluong modified the milestones: 89, 90 May 7, 2021
eliserichards pushed a commit that referenced this issue May 7, 2021
@neha-b2001
For #8984: Added color for "Share" menu's "Recently Used" in Dark the…
0eaf9f3 
…me (#18839)

* Added color for "Share" menu's "Recently Used" in Dark theme

- Changed HEX code for inset_dark_theme in values/colors.xml

* Replaced #52525E in values/colors.xml with @color/photonDarkGrey10

* Added color for "Share" menu's "Recently Used" in Dark theme

- Created a separate attribute recentlyUsedSharedMenu to be used by 'recently used' panel of tab share menu
- It specifies colors to be used for Light and Dark theme
@eliserichards eliserichards reopened this May 7, 2021
@eliserichards eliserichards added the eng:qa:needed QA Needed label May 7, 2021
@gabrielluong gabrielluong added this to Ready for Engineering (min-5 ; max-22) in Android Engineering Team Kanban board via automation May 8, 2021
@gabrielluong gabrielluong moved this from Ready for Engineering (min-5 ; max-22) to In Testing/QA in Android Engineering Team Kanban board May 8, 2021
@LaurentiuApahideanSV
Copy link

LaurentiuApahideanSV commented May 10, 2021 •
edited

Verified as fixed on Firefox Preview Nightly 90.0a1 (Build #2015809515) 5/10.

Devices used:

  • OnePlus 6T (Android 9)
  • Google Pixel 3 XL (Android 9)

@LaurentiuApahideanSV LaurentiuApahideanSV added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels May 10, 2021
Android Engineering Team Kanban board automation moved this from In Testing/QA to Done May 10, 2021
pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this issue Aug 10, 2021
@neha-b2001 @pkirakosyan
For mozilla-mobile#8984: Added color for "Share" menu's "Recently Use…
2465256 
…d" in Dark theme
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@neha-b2001 neha-b2001

Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Themes Dark mode, light mode, private browsing mode good first issue Good for newcomers help wanted Help wanted from a contributor. More complex than good first issue. S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist
Projects
No open projects
Android Engineering Team Kanban board
Done
Milestone
90
Development

Successfully merging a pull request may close this issue.

For #8984: Added color for "Share" menu's "Recently Used" in Dark theme neha-b2001/fenix
9 participants
@darkwing @gabrielluong @ZianeA @Cheap-Skate @eliserichards @mcarare @LaurentiuApahideanSV @sflorean @neha-b2001