[4.0][Atum] Refactor CSS variables and template options #26440

ciar4n · 2019-09-30T11:07:56Z

Pull Request for Issue # .

Summary of Changes

Reduces defined CSS variables. atum-bg-dark variables combined to a single primary variable
Remove the need for template php color helper file
Refactors template options. Changes 'primary color' from hue format. Renames 'special' to 'secondary'
Remove fill from logo SVG. Applies primary color to logo
Removed some unneeded CSS declarations (removed or changed to inherit)
Made template color options reactive. Template colors change as you change the input....

Testing Instructions

Apply this patch and run node build.js --compile-css and node build.js --compile-js to update the changed SCSS and JS. Alternatively, you can run npm i.

Navigate to admin template style settings. Check color options. General color check across template.

Documentation Changes Required

build/media_source/templates/atum/js/template.es6.js

…efactor-atum-color

wilsonge · 2019-09-30T12:47:42Z

@bembelimen are you able to take a look through this and make sure nothing on your guys side has regressed in the template options?

administrator/language/en-GB/en-GB.tpl_atum.ini

…a-cms into refactor-atum-color

wilsonge · 2019-09-30T22:07:45Z

The system tests failing here are actually due to this PR.

141 | 18:41:49.747 SEVERE - http://localhost/test-install/media/templates/atum/js/template.min.js?b3f9923a4b6a60cad8a308ec588d0a0b 0:4691 Uncaught TypeError: Cannot set property 'onchange' of null
142 | 18:41:49.990 SEVERE - http://localhost/test-install/media/templates/atum/js/template.min.js?b3f9923a4b6a60cad8a308ec588d0a0b 0:4691 Uncaught TypeError: Cannot set property 'onchange' of null
143 | 18:41:51.382 SEVERE - http://localhost/test-install/media/templates/atum/js/template.min.js?b3f9923a4b6a60cad8a308ec588d0a0b 0:4691 Uncaught TypeError: Cannot set property 'onchange' of null
144 | 18:41:53.661 SEVERE - http://localhost/test-install/media/vendor/joomla-custom-elements/js/joomla-alert.min.js?b3f9923a4b6a60cad8a308ec588d0a0b 0:1742 Uncaught TypeError: Cannot read property 'removeChild' of null
145 | 18:41:55.54 SEVERE - http://localhost/test-install/media/templates/atum/js/template.min.js?b3f9923a4b6a60cad8a308ec588d0a0b 0:4691 Uncaught TypeError: Cannot set property 'onchange' of null

The removeElement thing is fine - it's not due to this PR but the onchange is - I assume we need to wrap and if statement around if the element being retrieved by ID is actually found on the page.

ciar4n · 2019-10-01T07:55:12Z

Thank you. Should be good now.

ciar4n · 2019-10-01T09:50:23Z

@wilsonge Assuming ozdemirburak/iris is not used elsewhere, I guess I can remove the following from composer.json ? ... https://github.com/joomla/joomla-cms/blob/4.0-dev/composer.json#L84

brianteeman · 2019-10-21T12:33:37Z

It is really simple - you cannot achieve the intended aim of enforcing an accessible colour scheme using the hue field AND have the other field selects. It is one or the other.

coolcat-creations · 2019-10-21T12:34:59Z

completely agree on that, that was never the intention back then to have the other selections...

ciar4n · 2019-10-22T00:00:38Z

I'm not sure where to go with this. I can delete all the color options, leaving it open for someone to create different themes as suggested by @coolcat-creations and @nadjak77? This PR still cleans up the CSS and with a few additional changes would makes theming easier.

Alternatively I leave as is. If we want a user to be able to define colors in the template options then this PR offers a CSS only, slimmer, client side solution. As suggested by @dgrammatiko this can probably be extended with JS but personnally I believe that is not required.

Lastly of course I can just close it.

brianteeman · 2019-10-22T20:49:42Z

Unfortunately as the template team did most of their work through direct commits there is no pull request which documents the intent of that hue field. I do note however that it was added to the template when it already had those other colour fields so it does seem odd to me that it wasn't considered that they cannot work together.

dgrammatiko · 2019-10-22T21:24:10Z

Many moons ago I did a small experiment to prove that we are all doing it wrong!
The code is public: https://github.com/dgrammatiko/scss-wasm-compile
What it does?
It compiles the template css from the source scss in the browser. So on install/update the template could recompile given any number of scss variables (colours or whatever). The only thing that is missing is a form that will store the scss variables in the db.

Did I mention that PHP is not the right tool for css works? I did, but let me repeat, PHP is not the right tool for scss/css works

wilsonge · 2019-11-25T11:06:07Z

OK so given we're having the chat here. What about if we decided to flag to the user in the interface if their chosen colours have a11y compliant contrasts or not (at the various levels). Because @dgrammatiko is right in the context we really shouldn't be compiling this after the params are saved. At worst it should be done at the point where we save the params (with JS if required).

My recollection of this discussions is that initially the values were supposed to be precomputed. However at a later point the decision was made in order to support company colour schemes that may not be a11y by default people could choose to manually override the colour schemes to create their own branded themes that obeyed a brand manual that wasn't a11y by default. However for most people the intention was they'd only need to set primary/secondary colour styles

brianteeman · 2019-11-25T11:52:45Z

Not sure what you mean about flagging up? Something like https://whocanuse.com/ ?

wilsonge · 2019-11-25T11:58:19Z

Yup something exactly like that obviously less items in the list than that to not make the UI too complicated. So by default we're accessible (and can achieve this in JS etc). If people override the schema then we can show them what QA they are/aren't meeting

brianteeman · 2019-11-25T12:02:12Z

No idea how to do that but at least I understand what you mean

dgrammatiko · 2019-11-25T19:01:05Z

At some point I saw in my tweeter feed a very nice solution for this exact problem. I just need to find it

EDIT: found it: https://twitter.com/kevgski/status/982714455300583424?s=21

C-Lodder · 2019-11-26T07:45:09Z

There it is! Spent ages trying to find that

brianteeman · 2019-11-26T08:26:53Z

that looks perfect @dgrammatiko

dgrammatiko · 2019-11-26T09:18:46Z

@brianteeman hm, it seems overkill for the task tho. Let me explain it has a hard dependency
to d3 (gzipped 150% the size of jQuery). This can be implemented with waaaaay less code, eg https://github.com/dequelabs/axe-core/blob/develop/lib/checks/color/color-contrast.js
This is from the axe library that G-lighthouse is using for the a11 tests.
Conceptually the task in hand is pretty simple: get the relationship between the inputs (foreground, background) and constantly check their values (printing the contrast ratio as a label or whatever)

wilsonge · 2019-11-26T10:01:47Z

D3 is a large library and does enough major version updates could be a bit of a concern :/ However practically speaking we need something that's not going to take forever to implement bearing in mind we have only a handful of quality javascript contributors to the CMS...

dgrammatiko · 2019-11-26T10:07:13Z

However practically speaking we need something that's not going to take forever to implement

FYI that demo doesn't reflect to a oss repo

brianteeman · 2019-11-26T10:30:02Z

The code in the demo is at https://github.com/KevinGutowski/KevinGutowski.github.io

As there is no licence listed we cannot use it (unless we ask - which is worth it I think)

brianteeman · 2019-11-26T10:32:05Z

I don't care how it is done technically but I am sick of hearing that it can be done better if .... and then it stalls because there is no one with the time/skill/will/energy to do it.

ciar4n · 2020-02-14T09:56:37Z

Do I close this?

wilsonge · 2020-02-16T13:39:09Z

Let's go for this and use the library @dgrammatiko is suggesting to feedback to users about whether their colours meet a11y requirements

brianteeman · 2020-02-16T13:43:16Z

perhaps wait until @angieradtke has finished??

wilsonge · 2020-02-16T15:15:46Z

yes we should wait on that

ciar4n · 2020-02-16T15:30:35Z

perhaps wait until @angieradtke has finished??

If you are gonna merge this, it should probably be done before any major work. My initial reason for this PR was to clean up the atum scss before an attempt to repaint it.

angieradtke · 2020-02-17T07:16:41Z

What do you mean exactly with clean up ? Von meinem iPhone gesendet

…

Am 16.02.2020 um 16:30 schrieb Ciaran Walsh ***@***.***>: perhaps wait until @angieradtke has finished?? If you are gonna merge this, it should probably be done before any major work. My initial reason for this PR was to clean up the atum scss before an attempt to repaint it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

ciar4n · 2020-02-17T09:27:36Z

Reduces defined CSS variables. atum-bg-dark variables combined to a single primary variable
Remove the need for template php color helper file
Refactors template options. Changes 'primary color' from hue format. Renames 'special' to 'secondary'
Remove unneeded CSS declarations

angieradtke · 2020-02-17T09:34:50Z

@ciar4n

Does anybody know why this is so complicated?
Why is the background not at the body-element ?
What did I miss?

body .container-main {
  position: relative;
  min-height: calc(100vh - #{$header-height} - 3px);
  padding-left: 0;
  padding-right: 0;
  overflow: auto;
  &::before,
  &::after {
    content: '';
    width: 100%;
    height: 100%;
    position: fixed;
    top: 0;
    left: 0;
    z-index: -1;
Use the ../images/joomla-pattern.svg through https://yoksel.github.io/url-encoder/
 background: var(--atum-bg-light) url("data:image/svg+xml,%3C%3Fxml version='1.0' ...... ;
    background-size: 15rem;
  }

  &::after {
  /*  background: linear-gradient(135deg, var(--atum-bg-light) 0%, var(--atum-bg-light) 20%, rgba(225, 229, 231, 0) 100%); */
  }
}

ciar4n · 2020-02-17T09:52:31Z

@angieradtke

Look like it is to apply both an SVG and a gradient to the background. As each use background-image property, to apply both they had been moved to pseudo-elements.

If you are removing either the SVG or gradient in your design then you can remove all this and just apply to .container-main. I'm sure the gradient can go as it is barely noticeable.

ciar4n · 2020-02-23T16:49:30Z

Closing. Now 5 months behind. To much work involved to bring up to date.

ciar4n added 10 commits September 27, 2019 18:04

remove inline fill values from logo svg

39ae956

refactor css variables

3ad3f9d

reactive color inputs

bc30e1c

set css vars from params

76cc202

remove template helper

57a00ad

update xml and lang

efcebbd

missed

9330834

apply primary color to logo

5826aa2

make remaining inputs reactive

f0b987f

remove redundant variables

0876fdc

joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Sep 30, 2019

hound bot reviewed Sep 30, 2019

View reviewed changes

hound

1f2ae47

hound bot reviewed Sep 30, 2019

View reviewed changes

ciar4n added 2 commits September 30, 2019 12:34

hound

600535b

Merge branch '4.0-dev' of https://github.com/joomla/joomla-cms into r…

4014121

…efactor-atum-color

ciar4n and others added 2 commits September 30, 2019 15:04

Merge branch '4.0-dev' into refactor-atum-color

26d8c6e

Codestyle

9e14ff5

Quy reviewed Sep 30, 2019

View reviewed changes

administrator/language/en-GB/en-GB.tpl_atum.ini Outdated Show resolved Hide resolved

ciar4n added 2 commits September 30, 2019 17:05

alpha sort lang

dd178d1

Merge branch 'refactor-atum-color' of https://github.com/ciar4n/jooml…

01a45b3

…a-cms into refactor-atum-color

Update template.es6.js

6fc70c0

ciar4n added 2 commits October 1, 2019 08:56

Update template.es6.js

d6e82a0

Merge branch '4.0-dev' into refactor-atum-color

cd09b7d

richard67 mentioned this pull request Nov 10, 2019

[4.0] Use HttpFactory to fetch zip file from CI server joomla-extensions/patchtester#249

Closed

ciar4n mentioned this pull request Nov 24, 2019

[4.0][a11y] Allow enabling of both 'High Contrast' & 'Monochrome' #27142

Merged

ciar4n closed this Feb 23, 2020

Uh oh!

[4.0][Atum] Refactor CSS variables and template options #26440

[4.0][Atum] Refactor CSS variables and template options #26440

Uh oh!

Conversation

ciar4n commented Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Testing Instructions

Documentation Changes Required

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wilsonge commented Sep 30, 2019

Uh oh!

Uh oh!

wilsonge commented Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ciar4n commented Oct 1, 2019

Uh oh!

ciar4n commented Oct 1, 2019

Uh oh!

brianteeman commented Oct 21, 2019

Uh oh!

coolcat-creations commented Oct 21, 2019

Uh oh!

ciar4n commented Oct 22, 2019

Uh oh!

brianteeman commented Oct 22, 2019

Uh oh!

dgrammatiko commented Oct 22, 2019

Uh oh!

wilsonge commented Nov 25, 2019

Uh oh!

brianteeman commented Nov 25, 2019

Uh oh!

wilsonge commented Nov 25, 2019

Uh oh!

brianteeman commented Nov 25, 2019

Uh oh!

dgrammatiko commented Nov 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

C-Lodder commented Nov 26, 2019

Uh oh!

brianteeman commented Nov 26, 2019

Uh oh!

dgrammatiko commented Nov 26, 2019

Uh oh!

wilsonge commented Nov 26, 2019

Uh oh!

dgrammatiko commented Nov 26, 2019

Uh oh!

brianteeman commented Nov 26, 2019

Uh oh!

brianteeman commented Nov 26, 2019

Uh oh!

ciar4n commented Feb 14, 2020

Uh oh!

wilsonge commented Feb 16, 2020

Uh oh!

brianteeman commented Feb 16, 2020

Uh oh!

wilsonge commented Feb 16, 2020

ciar4n commented Sep 30, 2019 •

edited

Loading

wilsonge commented Sep 30, 2019 •

edited

Loading

dgrammatiko commented Nov 25, 2019 •

edited

Loading