High Contrast Light Theme #3819

tessgadwa · 2017-05-05T16:09:05Z

Configuration and CSS files for option high contrast condensed light Riot desktop theme.

Usersettings.js in matrix-react-sdk must also be updated in order for the theme to be accessible from the Settings menu.

Signed-off-by: Tess Gadwa [email protected]

lukebarnard1 · 2017-05-05T16:59:37Z

I've had a quick play with this, and it's generally looking good. I'll review the code in a bit (the changes to webpack config are unexpected).

There were three things that I noticed (feel free to ignore the black outlines, my screenshot thing is a bit crap):

Redacted messages look like they're from the dark theme
Undecryptable messages appear mis-aligned
Likewise for membership events

I also noticed that the line-height of messages has been decreased and I'm not sure if that was intentional:

dbkr

A few comments on the code side from me

dbkr · 2017-05-05T16:58:26Z

webpack.config.js


 module.exports = {
    entry: {
        "bundle": "./src/vector/index.js",
-        "indexeddb-worker": "./src/vector/indexedbd-worker.js",


I'm assuming this and the above is a merge gone wrong: this will break the indexeddb worker.

Webpack.config.js has been updated; other changes forthcoming.

dbkr · 2017-05-05T16:58:50Z

webpack.config.js

    },
    module: {
        preLoaders: [
-            { test: /\.js$/, loader: "source-map-loader" },
+            { test: /\.js$/, loader: "source-map-loader" }


This change also presumably wasn't intentional?

dbkr · 2017-05-05T17:00:06Z

webpack.config.js


            // olm takes ages for webpack to process, and it's already heavily
            // optimised, so there is little to gain by us uglifying it.
-            /olm[\\\/](javascript[\\\/])?olm\.js$/,
+            /olm\/(javascript\/)?olm\.js$/,


Was this not necessary?

yeah the [\\\/] is needed for os interoperability

dbkr · 2017-05-05T17:00:53Z

webpack.config.js

+            {
+                allChunks: true
+            }
+        ),


The previous formatting was correct according to our code style.

tessgadwa · 2017-05-05T17:03:47Z

Line height of messages was decreased, and very much intentional! A more condensed display seems to be one of the things people are asking for, even more so than high contrast.

lukebarnard1

The changes to webpack appear to be reverting something else. I would go and get the latest webpack.config from develop and then apply your one-line change to it.

lukebarnard1 · 2017-05-05T17:00:48Z

src/skins/vector/css/themes/contrastlight.scss

@@ -0,0 +1,4 @@
+@import "_base.scss";
+@import "_dark.scss";


I guess this explains why redacted messages appear to come from the dark theme! Perhaps this should be _light ?

lukebarnard1 · 2017-05-05T17:02:38Z

src/skins/vector/css/themes/_contrastlight.scss

+    color: rgba(0, 0, 0, 1) !important;
+    font-weight:600;
+    opacity: 1  !important;
+    }


There should be an empty line at the bottom here, and that } should not have a tab before it

lukebarnard1 · 2017-05-05T17:03:05Z

src/skins/vector/css/themes/_contrastlight.scss

+.mx_SearchBox_search {
+color: rgba(0, 0, 0, 1) !important;
+font-weight: bold !important;
+opacity: 1 !important;


The above three lines need indenting.

tessgadwa · 2017-05-05T17:06:13Z

Line height of messages was decreased, and very much intentional! A more condensed display seems to be one of the things people are asking for, even more so than high contrast.

lukebarnard1 · 2017-05-05T17:07:22Z

one of the things people are asking for, even more so than high contrast

I think we should separate the two concerns in that case. I think having an optional "condensed" mode is more what people are after (despite me not being able to find that GH issue).

tessgadwa · 2017-05-05T18:44:07Z

Hey Luke,

Thanks for the thorough code review; I'll plan to make these changes over the weekend. I am all for separating out high contrast and condensed themes -- I'll see if I can pull up any of the convo threads from DesignUX that asked for this.

Based on feedback from Alan Day and others, refined the theming submitted in the original PR submitted March 14 to: - Make body text less heavy - Condense vertical spacing of text

File update to add two new Riot desktop themes: high contrast and high contrast condensed text. Signed-off-by: Tess Gadwa [email protected]

CSS files for two new Riot desktop themes: high contrast and high contrast condensed. Usersettings.js in matrix-react-sdk must also be updated in order for the theme to be accessible from the Settings menu. Signed-off-by: Tess Gadwa [email protected]

tessgadwa · 2017-05-10T05:52:15Z

Updated files to create two new themes:

Light High Contrast
Light High Contrast Condensed

LHC differs only from the current theme in color and font-weight; LHC-Condensed includes the same changes and addresses several user' concerns that there is too much whitespace.

ara4n · 2017-05-15T00:49:37Z

@tessgadwa is this ready for more review?

tessgadwa · 2017-05-22T03:48:44Z

@ara4n Just saw this. Yes, it's ready for more review.

t3chguy · 2017-08-02T06:50:08Z

bump, @lukebarnard1 / @ara4n

uhoreg · 2017-09-05T20:40:37Z

Is the condensed version still needed now that we have the "compact timeline layout" setting?

tessgadwa · 2017-09-05T20:54:12Z

My understanding is that the introduction of this setting removed the need for a condensed version of the theme.

…

On Tue, Sep 5, 2017 at 1:40 PM, Hubert Chathi ***@***.***> wrote: Is the condensed version still needed now that we have the "compact timeline layout" setting? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3819 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGLNG1WygNNw-6qjK5NI7ePz3xWoOt0Aks5sfbHIgaJpZM4NSHht> .

-- Tess Gadwa Chief Executive Officer

_______________________________ Yes Exactly, Inc. | yesexactly.com | 413.325.8251

lukebarnard1 · 2017-09-18T14:55:17Z

Ok, @tessgadwa looks like you'll need to remove the condensed version and just leave the light contrast theme in.

tessgadwa · 2017-09-18T15:24:31Z

@lukebarnard1 is this still an active PR? If so I will certainly make the updates.

lukebarnard1 · 2017-09-18T16:25:05Z

is this still an active PR

Sure, unless the light contrast theme has already made it in, which it doesn't look like it.

t3chguy · 2017-12-27T20:33:46Z

In an attempt to revive this:

The condensed theme needs removing as this is being done beside themes in the "Compact Mode" as per High Contrast Light Theme #3819 (comment)

lukebarnard1 · 2018-06-25T13:20:44Z

@tessgadwa given that we're about to go through a redesign of Riot with the intention of improving the contrast of colours in the app, I think we'll close this and the other PR.

Thanks anyway for the contribution 👍

tessgadwa · 2018-06-25T16:13:29Z

Thanks for the note. I thought it was closed a long time ago.

lukebarnard1 mentioned this pull request May 5, 2017

Adds Option to Choose Light High Contrast Theme matrix-org/matrix-react-sdk#865

Closed

dbkr reviewed May 5, 2017

View reviewed changes

dbkr assigned ara4n May 5, 2017

dbkr changed the title ~~Develop~~ High Contrast Light Theme May 5, 2017

lukebarnard1 suggested changes May 5, 2017

View reviewed changes

High Contrast Light Theme for Riot

cbcb830

Based on feedback from Alan Day and others, refined the theming submitted in the original PR submitted March 14 to: - Make body text less heavy - Condense vertical spacing of text

tessgadwa force-pushed the develop branch from d749d0f to cbcb830 Compare May 7, 2017 08:01

tessgadwa added 2 commits May 9, 2017 22:38

Add files via upload

f723b74

File update to add two new Riot desktop themes: high contrast and high contrast condensed text. Signed-off-by: Tess Gadwa [email protected]

Add files via upload

7d11721

CSS files for two new Riot desktop themes: high contrast and high contrast condensed. Usersettings.js in matrix-react-sdk must also be updated in order for the theme to be accessible from the Settings menu. Signed-off-by: Tess Gadwa [email protected]

lukebarnard1 closed this Jun 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

High Contrast Light Theme #3819

High Contrast Light Theme #3819

tessgadwa commented May 5, 2017

lukebarnard1 commented May 5, 2017

dbkr left a comment

dbkr May 5, 2017

tessgadwa May 7, 2017

dbkr May 5, 2017

dbkr May 5, 2017

t3chguy May 5, 2017

dbkr May 5, 2017

tessgadwa commented May 5, 2017 •

edited

Loading

lukebarnard1 left a comment

lukebarnard1 May 5, 2017

lukebarnard1 May 5, 2017

lukebarnard1 May 5, 2017

tessgadwa commented May 5, 2017 •

edited

Loading

lukebarnard1 commented May 5, 2017

tessgadwa commented May 5, 2017 •

edited

Loading

tessgadwa commented May 10, 2017 •

edited

Loading

ara4n commented May 15, 2017

tessgadwa commented May 22, 2017

t3chguy commented Aug 2, 2017

uhoreg commented Sep 5, 2017

tessgadwa commented Sep 5, 2017 via email

lukebarnard1 commented Sep 18, 2017

tessgadwa commented Sep 18, 2017

lukebarnard1 commented Sep 18, 2017

t3chguy commented Dec 27, 2017

lukebarnard1 commented Jun 25, 2018

tessgadwa commented Jun 25, 2018

High Contrast Light Theme #3819

High Contrast Light Theme #3819

Conversation

tessgadwa commented May 5, 2017

lukebarnard1 commented May 5, 2017

dbkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tessgadwa commented May 5, 2017 • edited Loading

lukebarnard1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tessgadwa commented May 5, 2017 • edited Loading

lukebarnard1 commented May 5, 2017

tessgadwa commented May 5, 2017 • edited Loading

tessgadwa commented May 10, 2017 • edited Loading

ara4n commented May 15, 2017

tessgadwa commented May 22, 2017

t3chguy commented Aug 2, 2017

uhoreg commented Sep 5, 2017

tessgadwa commented Sep 5, 2017 via email

lukebarnard1 commented Sep 18, 2017

tessgadwa commented Sep 18, 2017

lukebarnard1 commented Sep 18, 2017

t3chguy commented Dec 27, 2017

lukebarnard1 commented Jun 25, 2018

tessgadwa commented Jun 25, 2018

tessgadwa commented May 5, 2017 •

edited

Loading

tessgadwa commented May 5, 2017 •

edited

Loading

tessgadwa commented May 5, 2017 •

edited

Loading

tessgadwa commented May 10, 2017 •

edited

Loading