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

Issue#2010 Switch to white or black background #2747

Closed
wants to merge 4 commits into from

Conversation

bcowgill
Copy link

#2010
body class=mocha-light or mocha-dark added based on
cookie mocha-scheme setting if found.
A little button added to bottom of test page which will allow the user to change the setting easily.

mochajs#2010
body class=mocha-light or mocha-dark added based on
cookie mocha-scheme setting if found.
A little button added to bottom of test page which will allow the user to change the setting easily.
@jsf-clabot
Copy link

jsf-clabot commented Mar 19, 2017

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.2%) to 88.188% when pulling c314b1f on bcowgill:master into b4ebabd on mochajs:master.

@bcowgill
Copy link
Author

The AppVeyor failure on node 0.12.18 doesn't happen for me locally.
mocha (master % u=)$ bin/mocha --inspect test/acceptance/http.spec.js
/home/me/.nvm/versions/node/v0.12.18/bin/node: bad option: --inspect
mocha (master % u=)$ bin/mocha test/acceptance/http.spec.js

http
✓ should provide an example

1 passing (19ms)

@ScottFreeCode
Copy link
Contributor

(Does anybody know a way to get AppVeyor to actually show the test failure result details? I am only seeing that the test failed, but not what the error/failure was.)

I'm not sure why we would need something like this built in when it's straightforward just to add custom CSS to the HTML (which is part of the project using Mocha rather than part of Mocha) or even to re-use that custom CSS by including it by a <link> to a file that's shared in a repo or package. That being said, if we want to consider it then the following issues would need to be addressed:

  • I don't get anything unless I include JQuery on the page. Doesn't seem like it's being used for anything that isn't easy to do with the DOM APIs (however much many developers may dislike them). I think we should not require projects using Mocha to add JQuery for this, and should just use the DOM APIs considering Mocha doesn't already have JQuery bundled.
  • The symbol is small and almost invisible by default, and frankly doesn't do a very good job of conveying its purpose before I click it even when I do see it and hover over it.
  • The scheme does not correctly change the background color back when it is dark and I click the button to change back to light (some other things do change back, leading to unreadable results such as black text on black background).
  • The root mocha.js file will be overwritten when the bundle is updated; code changes would need to go into the source files.

@bcowgill
Copy link
Author

ScottFreeCode, thanks for your comments.
What browser did you try it on?
Did you use file:// or an http:// protocol when loading?
I don't see the usage issues you are reporting on Chrome.

Most of it can be done with CSS except the top right progress circle because it uses a canvas you cannot change the fillStyle with CSS.

I based the opacity to match the other |> symbols but can certainly make it more visible.

mochajs#2010
Removed user interface for the moment.
body class=mocha-light or mocha-dark added based on
cookie mocha-scheme setting if found.

You can activate by adding mocha-dark class to body of your html test runner.

Then once you run the tests the cookie is set for a year and you can remove the
body class and your preference is retained.

This way different team members can have their light/dark setting retained without
affecting others.
@bcowgill
Copy link
Author

bcowgill commented Mar 20, 2017

ok, I've removed the UI toggler for now and just use the color scheme setting from the cookie.
I've been waiting quite a while for this so I hope we can get this in.

To test it I just added class="mocha-dark" to the body tag of my HTML runner and ran the page.
After that deleting the class still keeps the dark scheme by reading the cookie.

I'm not sure where the documentation lies for this:
https://mochajs.org/#running-mocha-in-the-browser
Or I would put a simple one-liner entry about this there.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.2%) to 88.188% when pulling f695b64 on bcowgill:master into b4ebabd on mochajs:master.

mochajs#2010
Make the test replay button more visible in dark color scheme.
@coveralls
Copy link

Coverage Status

Coverage increased (+2.2%) to 88.188% when pulling 0704496 on bcowgill:master into b4ebabd on mochajs:master.

mochajs#2010
Moved color scheme code to html reporter which makes more sense.
@coveralls
Copy link

Coverage Status

Coverage increased (+1.3%) to 87.29% when pulling 8db4b08 on bcowgill:master into b4ebabd on mochajs:master.

@drazisil drazisil added the type: feature enhancement proposal label Mar 30, 2017
@boneskull
Copy link
Member

@bcowgill

Hi, thanks for the PR. I'm not sure why the JavaScript is necessary to implement a dark scheme. Simply using your own custom styles should work; there's no requirement to use mocha.css (or any CSS, really) to execute tests in the browser.

I'm disinclined to merge this, because it increases complexity and maintenance overhead to benefit a relatively small portion of our userbase (those executing browser-based unit tests in a non-automated fashion) . Mocha can't afford that.

I'd encourage you to create a gist or find another way to share your dark color scheme, as I'm sure others may find it useful. Also, there are plugins for browsers such as Dark Reader which you might find useful.

Anyway, even if this is not merged, please do not be discouraged from contributing to Mocha. 😄 You may have even created this PR or issue before we added contribution guidelines. Feature PRs are more likely to be merged if they arrive after discussion. There are plenty of other areas in which Mocha needs help, as you can see in the README!

@boneskull boneskull closed this May 12, 2017
@steve-taylor
Copy link

This is worth revisiting in another PR given widespread support for prefers-color-scheme. There's no need for any JavaScript or cookies.

@boneskull
Copy link
Member

@steve-taylor agree.

@bcowgill
Copy link
Author

@steve-taylor, @boneskull

It's been a long time, but I'm updating one of my projects that uses mocha and so am updating this again for mocha 8.3.0. And have a bit of time available.

The use of prefers-color-scheme CSS would be a nice (minimal) thing to add but people with vision difficulties may want to turn on light/dark schemes for things independently of changing their entire OS color scheme.
For example, there are still a lot of apps that do not render well when the OS dark scheme is set as they hard code some colors to black still.
Sometimes vision problems are temporary (before / after cataract surgery, speaking from experience.)
Also, for example, I use Linux with i3 window manager which is a minimalist environment and don't think it even supports specifying the prefers-color-scheme.
Use of things like chrome extensions to invert colors is helpful but usually you get a wonky color scheme when you simply invert them.

For the best accessibility experience the user needs to be able to switch between light and dark as needed.
Adding a mocha-dark class to the body in the test page index.html is a good compromise.
Normal team members can use the usual index.html page and another team member can simply use a git stash to apply the class to the body when they are running the tests or keep an index-dark.html alongside to choose their scheme.
But ideally the user would want to be able to click something on the page to just toggle the color scheme and have it be remembered when they refresh.
In my local changes, I've used the circular progress indicator as the place to click to toggle the scheme and use localStorage to save the setting only once the user has done so.
So if the CSS prefers is present it will work and if the user overrides the setting in the body or by clickin it will work.

A Javascript change is also needed to properly effect the color scheme change because the circular mocha progress indicator is rendered in a canvas and cannot be seen when you go to dark theme. (see screenshot)

mocha-light-dark-canvas

So at the very least, even if prefers-color-scheme is used the canvas code must be made aware of that to change the text color.
If the click to toggle is present it needs to re-render the progress indicator to correct the rendered colors of text against the background.

We could get by with a fixed text color of orange or blue for the progress indicator which is pretty visible against both white and black. (obviously we wouldn't want to use green or red as those have semantic meaning about passing and failing.)

I can create a fresh pull request which switches the scheme based on prefers-color-scheme and a mocha-dark class name if you're willing.

Alternatively I'll create a mocha-dark package on NPM and include the css and javascript file there with full functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature enhancement proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants