Closed Bug 1187696 Opened 9 years ago Closed 5 years ago

Reader View sidebar (and perhaps the settings panel?) is too "bright" in dark mode

Categories

(Toolkit :: Reader Mode, defect, P3)

Product:
Toolkit
Component:
Reader Mode
Version:
42 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox68 --- fixed

People

(Reporter: supaiku, Assigned: akshithashetty84)

References

Details

(Whiteboard: [about-reader-ui] [bugday-20190814])

Attachments

(2 files)

ReaderViewPanelOnDarkBackground.png
74.01 KB, image/png
Details
Bug 1187696 - Toolbar too bright in Reader dark mode
47 bytes, text/x-phabricator-request
Details | Review 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20150725030209

Steps to reproduce:

1. Enter reader view
2. Set background to dark




Actual results:

The panel on the right stays white. See attachment.


Expected results:

The panel should adopt the color of the background, because the way it's now, it's too distracting when reading on a dark background color.
Component: Untriaged → Reader Mode
Product: Firefox → Toolkit
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Summary: Reader View panel is too "bright" in dark mode → Reader View sidebar (and perhaps the settings panel?) is too "bright" in dark mode
Whiteboard: [about-reader-ui]

Is there a chance that I can be assigned to this Issue please?

(In reply to Victor from comment #3)

Is there a chance that I can be assigned to this Issue please?

This isn't a mentored bug... are you working on a patch for this bug?

Flags: needinfo?(vivee18)

yes I am working on a patch sir. But I hope I can continue?

Flags: needinfo?(vivee18)

(In reply to Victor from comment #5)

yes I am working on a patch sir. But I hope I can continue?

I'm always happy with patches. But I'd like to understand how you intend to fix this. Can you outline what your plan is?

I'll note that there's a plan to do a Google Summer of Code project around this code area, see https://wiki.mozilla.org/Community:SummerOfCode19 . Based on those UX mockups ( https://mozilla.invisionapp.com/share/87N9YQYCTJZ#/screens/315073983_Reader_View_Redesign ) the code might change substantially, so I'm not sure if now is the best time to work on this issue; the code might have to be rewritten soon...

Flags: needinfo?(vivee18)

(In reply to :Gijs (he/him) from comment #10)

(In reply to zaraahmad2121 from comment #9)

Hi, I am an applicant for GSoC. I am currently in my senior year of BS-CS programme. I had like to work on this project "Firefox Reader Redesign". Could you please guide me and help me get started?

Victor has already indicated he wants to work on this bug. Maybe have a look at bug 1151735 instead.

(In reply to Berkay Barlas from comment #7)

Given this bug already had interest, perhaps look at bug 1151732.

Thanks for considering granting me the privilege, I also am a gsoc applicant but i was with thought that there was a person already working on the issue, I have tried checking @Mozilla-central repo and found this component (Reader.js) https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/Reader.js

However I am queit new to the large codebase and will need guidance to where i can look to be sure i am doing the right thing and to see if another was working already on it for gsoc, Thanks for reading too. I'm quiet ready for your reply to get started.

Flags: needinfo?(vivee18)

(In reply to Victor from comment #12)

Thanks for considering granting me the privilege, I also am a gsoc applicant but i was with thought that there was a person already working on the issue, I have tried checking @Mozilla-central repo and found this component (Reader.js) https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/Reader.js

However I am queit new to the large codebase and will need guidance to where i can look to be sure i am doing the right thing and to see if another was working already on it for gsoc, Thanks for reading too. I'm quiet ready for your reply to get started.

The CSS is at https://searchfox.org/mozilla-central/source/toolkit/themes/shared/aboutReader.css . If we wanted to add styling such that the sidebar also goes dark when the background goes dark, that's where it'd need to happen. We might need to also update the icons referenced there so they use bright foreground colours instead of dark ones when the background is dark.

Do you have a copy of Firefox built from source already? If not, that'd be a good place to start. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds and https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build can help.

Hi Victor, I found some helpful things to simplify the large codebase for you. Might be of help :)

This is how I would have gone about it:

  1. To update the color of the toolbar: Try to figure out how the background change is triggered on the click of sepia/dark/light button. I found these separate classes body.sepia and body.dark which change the background colour in https://searchfox.org/mozilla-central/source/toolkit/themes/shared/aboutReader.css.
    I believe some new code has to be added under (Line 126):
    /* Override some controls and content styles based on colour scheme */
    :Gijs, Let me know if I am going wrong here.
  2. Change the icons/ add icons of a lighter color for dark mode, you can use Inkscape for that. They can be found using inspect element.
  3. Update the dropdown pop-up to match the color of the theme.
  4. The dropdown arrow has a white triangle, which looks weird when the dropdown popup has a different color. So you should make that triangle transparent.

:Gijs, In case Victor is busy or gives up working on the issue, then I would like to work on it. I am a GSoC applicant. Thanks! :)

Victor, any progress here? Are you stuck on anything I can help with?

Flags: needinfo?(vivee18)

(In reply to :Gijs (he/him) from comment #13)

The CSS is at https://searchfox.org/mozilla-central/source/toolkit/themes/shared/aboutReader.css . If we wanted to add styling such that the sidebar also goes dark when the background goes dark, that's where it'd need to happen. We might need to also update the icons referenced there so they use bright foreground colours instead of dark ones when the background is dark.

Do you have a copy of Firefox built from source already? If not, that'd be a good place to start. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds and https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build can help.

:Gijs (he/him), I assume one has to build entire Firefox source code to make changes(to .js files) in order to test it. Is this right or is there a faster way to go about it?

I have been tring to build Firefox for a few days now,

  1. I started off with trying to build it on my MacBook Air. Some errors are coming related to inttype.h not found. It seemed to be an error due to Mojave SDK, but even installing older SDK did not work. I did not have much luck in resolving the issue. I even tried asking for help on #build in Mozilla irc.
  2. Further, I tried it on a few other college systems with no luck.
  3. I moved on to trying it on GCP (The Windows VM always fail during hg clone, and with a Linux VM I fixed a few library issues and currently trying to build)
  4. I also tried Janitor
    I have not been able to do much development as of now. A lot of times the hg clone aborts in the middle. I was able to compile Firefox a couple of times on Janitor but now I ran into some Janitor specific bug.

I am confused on what would be the best way to go about this, could you please help?

(In reply to sonali18317 from comment #17)

:Gijs (he/him), I assume one has to build entire Firefox source code to make changes(to .js files) in order to test it. Is this right or is there a faster way to go about it?

You can use artifact builds, to download the binary (C++, Rust, some other) bits instead of compiling them. See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds . Basically, you want to edit your .mozconfig file (create one if you haven't yet) to specify the flags mentioned there, and then ./mach clobber and ./mach build, and that should be a lot easier, especially on your macbook air (you still need mozillabuild bits on windows).

For this bug though, I'd like to give Victor a chance to respond for a bit longer.

:Gijs, Thanks that helped a lot.
No problem, I am working on Bug 1151735

Hi,
Is this bug available to work on ? I would love to propose some suggestions for this in case no one has set their hands on it yet.
Thanks.

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to akshitha shetty from comment #20)

Hi,
Is this bug available to work on ? I would love to propose some suggestions for this in case no one has set their hands on it yet.
Thanks.

Sure. We probably want to use the same background for the sidebar as for the main page, and make sure the icons and text in the sidebar use the same foreground colour as the rest of the page. See also comment #6.

Flags: needinfo?(vivee18)
Flags: needinfo?(gijskruitbosch+bugs)

Thanks a lot for the direction.
So considering the mockups, the plan is to follow similar colors as proposed in the latest mockups but to to keep the icon design same as it is right now with only a change in the icon colors. What this will do is that it will keep it consistent with the existing toolbar in the light and sepia mode so that we get it working now as well as further improvise on it in the future months.
Let me know if this is this is right approach. Thank you.

(In reply to akshitha shetty from comment #22)

Thanks a lot for the direction.
So considering the mockups, the plan is to follow similar colors as proposed in the latest mockups but to to keep the icon design same as it is right now with only a change in the icon colors. What this will do is that it will keep it consistent with the existing toolbar in the light and sepia mode so that we get it working now as well as further improvise on it in the future months.
Let me know if this is this is right approach. Thank you.

Yep, this seems OK.

Hi, so I have worked around the css involved in solving the issue and am attaching a video of the output. (Note - The red hover on the close button is exactly the same as the present red and seems darker due to screen capturing). I have ensured that the styling is applied only for the dark mode.

Link to Video -
http://dw.convertfiles.com/files/0583129001554243057/simplescreenrecorder-2019-04-03_03.17.36.m4v

The next step would we to change the icons. I have noticed that the icon are in svg format. Could you please guide me as to how I should go about replacing these. I can create the icons myself in illustrator or any preferred software in needed. Would just need a little guidance here before I get started if any guidelines have to be maintained for the same.
Thanks for your effort on this.

Attachment #9056160 - Attachment description: Toolbar too bright in Reader dark mode → Bug 1187696 - Toolbar too bright in Reader dark mode
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8ab68eea15b8
Toolbar too bright in Reader dark mode r=Gijs

https://hg.mozilla.org/mozilla-central/rev/8ab68eea15b8

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → akshithashetty84
QA Whiteboard: [good first verify]

I have reproduced this bug with Nightly 42.0a1 (2015-07-26) on Windows 7, 64 Bit. The fix of this bug is verified with latest Firefox!

Build ID 20190717172542
User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Status: RESOLVED → VERIFIED
Whiteboard: [about-reader-ui] → [about-reader-ui] [bugday-20190814]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: