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

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.

Thank you. Should be good now.

@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

Yup if you just run composer remove ozdemirburak/iris. the entry should be removed from composer.json and also the lock file will get updated (i don't think it's used in other places but haven't checked - so please do :) )

Ty. I can't see it been used elsewhere so removed.

@ciar4n @wilsonge i think the color field type hue can be safely deleted as well (also the accompanying js, etc)

It's another concept of color calculating, so I guess I'm not the one to ask for.
@coolcat-creations and @nadjak77 and @chmst for a11y

The logo colour was removed in the PR requested by @C-Lodder somewhere here: #25570

As long as we're getting a11y compliant colours at the end it would be better to calculate them at save time once rather than every page load. However we need to ensure we're matching behaviour

Before and after this PR there is nothing stopping a user from selecting non a11y compliant colours. I think that is just a given.

The eventual intent is to create a PR where a user can select the type of vision impairment from a dropdown (dueteranopia, protanopia etc.). A pallette is then loaded to help that impairment.

To do so I would require this PR and a follow-up PR (porting css variables through bootstrap).

there is no such thing as a11y compliant colours - just contrasts

there is no such thing as a11y compliant colours - just contrasts

??... it is pretty obvious what the context is... colours that contrast in a11y compliant manner then.

I have tested this item ✅ successfully on 59a5b25

I have tested this item ✅ successfully on 59a5b25

A hint here, apart from the composer library the extra colour field should be removed as well (can be done in another PR of course)

I have tested this item ? unsuccessfully on 59a5b25

the calculation of the colors are wrong. the contrasts are not a11y.
f.e. use #0a0006 for primary color.
It is not easy to calculate the right color gradation. that is the reason for the hue value.
so you can calculate always the right contrasts for a11y

Applied the patch, recompiled JS and CSS.
But in the admin template styles, template color didn't on control change.
Tested unsuccessfully.
(Used Patch Tester 3, due to Patch Tester 4 didn't work)

I have tested this item ? unsuccessfully on 59a5b25

Thank you for the tests.

@nadjak77 Not a valid fail. A user can select non a11y colors prior to this PR. If a user been able to select no a11y colors is an issue, these options need to be removed entirely.

@rmittl You need to 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.

I applied the patch again.
Checked if the last changes are appeared.
Run node as described, no error.
But no changes in the template color and no JS error in the console.

@ciar4n How could I check the JS Event?

@ciar4n
That is right. A user can choose for color and background colors whitch have not the right contrast. But for the calculated colors he has no choice (only with some overrides in the css). He only can choose one color. so it has to be calculated the right ones.
And you can see in my screenshot, that there is nearly no difference between the colors.
And for the contrast color there is this error for nearly every color I choose in your patch.
We tried a really long time to calculate the colours the right way. We had the same errors like in your patch, so we decided to select only the hue value, to avoid the problem if someone choose a very light or very dark colour. The calculated colors always has to have the same "difference" between their gradation.

@Quy
Another point: why did you replaced yesterday the calculated colours with a gradient? You switch the colour to a darker colour (thats o.k.). but in your patch, if I choose a dark color for the dar-background (f.e. #0a0006) you cannot see that the colors get darker.

@nadjak77 So you want to switch back to using a hue field? You want to restrict the color selection to a very exact lightness and saturation? That comes with its own issues. The obvious one being that a user can not select their company colors exactly unless they just happen to have the right lightness and saturation. We can't restrict users like that. You have to assume they have some common sense..

It's for a11y, if you want to use own colors you can add a user.css

@coolcat-creations You can not restrict a color field so a user can only choose a select band of colors. If you honestly believe this then remove all colors fields. Even with your restrictions it still fails at stopping a user from selecting a non a11y color scheme....

if the reason was accessibility then it fails

I just wanted to clarify the initial intentions, I am longer not in the team so I don't know the current state.

To use a hex or a hue value is not the point of the discussion.
The point is, that the colors in your patch are wrong. If you can solve the error I have no problem to use a hex field.
I only want to explain, why we switched to the hue value.

Ah, and I saw another point:
You renamed the dark background color to primary, and the specialcolor to secondary. I think you should descripe, where the colors are used.

Do you use the secondary color as contrast color?
In my opinion that breaks the color concept and the ration for the contrast. The contrast color and the "special color" are not the same! Please, do not break the new color concept.
So the contrast color should be calculated.
You are right, that you can break the contrast ratio by choosing the colors. But it is "in your hand". But the calculated colors (and also the contrast color) you only can change with overrides in the custom.css. Many people cannot do overrides, so this colors have to be right calculated.

@nadjak77 can you explain how the hue created accessible colours please as I dont understand how that is possible. I get that if you only use the hue then accessibility contrast is maintained but as soon as you change any of the other colour fields then it can break.

@nadjak77

The point is, that the colors in your patch are wrong. If you can solve the error I have no problem to use a hex field.

Define what you mean by 'wrong'? If your issue is that when you select black (which was not even possible previously) as the primary color, the contrasts are not visible in the sidebar then that is easily fixed.

IMO 'Special' color means nothing. What is so special about it? Is it more special than the hue? You say 'The contrast color and the "special color" are not the same!'... how is anybody suppose to know that? The logic of a primary and secondary color is widely used and accepted (inc. Bootstrap).

I havn't done the calculation itself. So I cannot explain how we got the formulars.
Perhaps @coolcat-creations can?

The formula is fine if you only use hue.

The calculations of the main color were made with a tool that switches color brightness, hue and saturation in a natural curve. Switching more than one parameter (h,l,s) enables a better a11y and a harmonic scheme. The main color was picked manually and tested against other main colors if they still work for a11y. We tested tons of combinations, curves and main colors, annoyed all people that had to rework the calculations and finally came to a result that was a11y for every hue value.

During working out the color system we came accross some issues, like the one "what if a user wants to have black as main color". That does not work to be accessible, because all the submenu levels have to be AT LEAST lighter or darker to be visually seperateable for the user. So a user would need to define the colors of each mainlevel, which is less easy as just toggling the hue and switching monochrome on/off.

For any professional that wants to have a specific color scheme for example for a company and is ok with breaking or not breaking a11y on own risk, the user.css can be edited with ease. The backend template options should be just an easy slide to change the hue and be a guarantee that no a11y will be broken.

Someone posted a screenshot here of a broken green screen, I don't see it anymore? That result does absolutely not look like the intention and has to be fixed. The naming of the colors can be changed, I guess that were only placeholder names by the team.

So far...

It sounds like this has been completely over thought. The result still failing at what you were trying to achieve, a bunch of unnecessary code, a composer dependency and a user been only able to select 256 out of a possible 16,000,000+ colors.

Well than it can be improved, but there is no reason to drop it completely. A user can still create a user.css for own color schemes. The hue slide is there to enable an accessible theme in the core template. If it fails, it has to be fixed. Nothing more and nothing less.

Well than it can be improved, but there is no reason to drop it completely.

How many reasons do you need? From a code perspective, I can mention a lot more!

If it fails, it has to be fixed. Nothing more and nothing less.

Do you understand that once you have multiple color fields the issue you try to solve becomes unsolvable? Unless you want to restrict every color field. And if you do the complexity of this multiplies with every color field.

Well than it can be improved, but there is no reason to drop it completely.

How many reasons do you need? From a code perspective, I can mention a lot more!

If it fails, it has to be fixed. Nothing more and nothing less.

Do you understand that once you have multiple color fields the issue you try to solve becomes unsolvable? Unless you want to restrict every color field. And if you do the complexity of this multiplies with every color field.

Yes, that the other colors can be selected free is not right. I just had a chat with @nadjak77 on skype and she had the idea to provided fixed color schemes. In that case you can drop all the calculation stuff and just bring some color presets that work. For anything else th user can use user.css.

@coolcat-creations A subset of fixed colour schemes aren't good UX. There will be many people who don't require an accessible colour contrast or that may simple require different options. Those that don't shouldn't be made to edit a CSS file.

Just have an accessible contrast by default.

We tested tons of combinations, curves and main colors, annoyed all people that had to rework the calculations and finally came to a result that was a11y for every hue value.

@coolcat-creations Some things never change. After all that work, all the related issues and fixes, the work done in this PR... now you just want to replace it with a couple of static color schemes?? Any wonder this template has been going on for 4 years and why so many of us stopped contributing to it.

Ciaran, big no I don't do and defintely don't decide anything here, I only replied to the questions above what the intentions where. I even don't know what this PR is about because I am not in the team anymore, I only explained what work and thoughts have been around in the past. That was all and I am out again ;-)

@ciar4n @coolcat-creations please refer to the original Pr for the newly styled template. Check my comments, I already proposed a js library that is using exactly the same function names, so porting the very poorly php code (it runs on every request, that’s unacceptable) shouldn’t take more than 20mins.

That said the hue field was never tested with repeatable forms so that is also another failure there (I assume this just by reading the ids in the script).

Anyways my point is the same as when I was commenting on that template PR:

• php is not the right tool for this problem

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.

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

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.

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.

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

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

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

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

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

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

There it is! Spent ages trying to find that

that looks perfect @dgrammatiko

@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)

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...

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

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)

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.

Do I close this?

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

perhaps wait until @angieradtke has finished??

yes we should wait on that

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.

What do you mean exactly with clean up ?

Von meinem iPhone gesendet

Am 16.02.2020 um 16:30 schrieb Ciaran Walsh notifications@github.com:

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.

• 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

@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%); */
  }
}

@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.

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