cerner / cucumber-forge-desktop

Cucumber Feature Reports Made Easy
Apache License 2.0
16 stars 6 forks source link

Move dark mode toggle box to app Settings #61

Closed lendkhoa closed 3 years ago

lendkhoa commented 3 years ago

Summary

Move the dark-mode toggle box to the app setting instead of individual view setting in response to this conversation dark_mode_fix_location2

Due to the default option of darkmodejs background color is white (#fff). and the need to instantiate the Darkmode class variable independently from appSetting section to avoid unwanted side effect (shown below - output's content is not visible due to our old similar background color), the app's default background is now also white. I have tried many different ways to get around this it seems unlikely. Does anyone have any suggestions? dark_mode_color_issue2

TobiTenno commented 3 years ago

I'm actually starting to dislike this, as it doesn't seem like darkmode-ignore actually works, so it's messing around with styles outside of its scope

lendkhoa commented 3 years ago

yeah, I only realize this limitation after playing around with it. Might have to create an issue for them to look into.

TobiTenno commented 3 years ago

go ahead and make the issue and we can mark this as blocked behind that, since that's the major thing holding back me being willing to go for this, as it creates inconsistencies in the settings panel

lendkhoa commented 3 years ago

Reported issue

jkuester commented 3 years ago

Okay so I spent quite a bit of time today playing around with darkmode-js and could never get it to work any better than what you have here. :( It seems like that library is just not going to integrate very smoothly with our existing step.

Honestly, at this point I am thinking that the most simple way forwards might be to ditch darkmode-js and just use normal css. It should not be hard to toggle the colors in the output pane. Basically what I thinking is just add something like this to the index.html style block:

      body.dark {
        background-color:  #272525;
      }
      #output.dark, #output.dark td, #output.dark th, #output.dark pre {
        background-color:  #272525;
        color: #f1f1f1;
        border-color: #f1f1f1;
      }

Then in the render.js all you would need is to stick something like this into the initSettings function:

  const darkModeCheckBox = document.getElementById('darkMode');
  darkModeCheckBox.addEventListener('click', () => {
    document.getElementById('output').classList.toggle('dark');
    document.getElementById('appBody').classList.toggle('dark');
  });

That should use your checkbox in the appSettings to toggle the dark theme for the output pane while leaving the rest of the app alone.

jkuester commented 3 years ago

The other thing we need to do in the render.js is to handle persisting the user's "darkmode" configuration between sessions. (So the next time they open the app it remembers that they had darkmode enabled.) You can use the store variable to set/get the info from persistent memory.

You should be able to follow what we are already doing for the gherkinDialect setting.

lendkhoa commented 3 years ago

Agree, darkmodejs is not the best option here. Switch to using plain css instead. dark_mode_plain_css

TobiTenno commented 3 years ago

looks like you've got some conflicts