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

Add colorblind theme #3743

Merged
merged 2 commits into from May 22, 2019

Conversation

@theFeu
Copy link
Contributor

commented Apr 12, 2019

No description provided.

@theFeu theFeu requested a review from lippserd Apr 12, 2019

@lippserd
Copy link
Member

left a comment

Thanks for the PR and the theme. Looks goo to me 👍
Please squash your commits before merge so that you have one for the theme and one for the doc.

@theFeu theFeu force-pushed the feature/add-colorblind-theme branch from 86fee69 to e756070 May 14, 2019

@lippserd lippserd requested a review from nilmerg May 14, 2019

@lippserd lippserd added this to the 2.7.0 milestone May 14, 2019

@nilmerg
Copy link
Member

left a comment

White text on a bright background? Even visually impaired users can't read this, right?

Bildschirmfoto von 2019-05-14 11-22-54
Bildschirmfoto von 2019-05-14 11-26-17

@theFeu

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

As discussed with @lippserd :

We should probably take care of it, that the modules work well with themes in general - in the modules themselves.
Themes should only replace the colours and not add any specific code for each module. (As seen in 'solarized dark' already)

@theFeu theFeu force-pushed the feature/add-colorblind-theme branch 3 times, most recently from b279b03 to 593a36f May 14, 2019

@theFeu theFeu force-pushed the feature/add-colorblind-theme branch from 593a36f to f6500bb May 16, 2019

@nilmerg nilmerg self-requested a review May 21, 2019

@nilmerg
Copy link
Member

left a comment

The theme looks ok now. But please move the documentation's section about accessibility to the bottom where translation is mentioned. At the very top this doesn't quite fit into the order of the other sections.

@theFeu theFeu force-pushed the feature/add-colorblind-theme branch from f6500bb to ee9fa89 May 21, 2019

@theFeu

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

done!

Jennifer Mourek

@theFeu theFeu force-pushed the feature/add-colorblind-theme branch from ee9fa89 to e90c443 May 21, 2019

@nilmerg nilmerg merged commit 687c28a into master May 22, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nilmerg nilmerg deleted the feature/add-colorblind-theme branch May 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.