Please use the Pull Request-Template.

Please use the Pull Request-Template.

In my opinion red and green should not be used at all

Thank you for the feedback.
So should i use neutral color or use borders styling for the compare view ?

Hi @Nikhil-sinhaa, Thank you for your first time contribution.
From my POV I think this is a good starting. The colors here are currently not the best, your suggestion is better but it's not my opinion to decide though.

Anyways, this PR is a feature and it should be rebased to 6.2.
5.4 is no longer accepting features, only bug fixes.

You can also close this PR and open another one against 5.4 if it would be easier for you.
Also, you can add the target branch name in the PR title like this: [6.2] Fix dark mode color contrast.

Thanks again!

Hi @Nikhil-sinhaa, Thank you for your first time contribution.
From my POV I think this is a good starting. The colors here are currently not the best, your suggestion is better but it's not my decision though.

Anyways, this PR is a feature and it should be rebased to 6.2.
5.4 is no longer accepting features, only bug fixes.

You can also close this PR and open another one against 5.4 if it would be easier for you.
Also, you can add the target branch name in the PR title like this: [6.2] Fix dark mode color contrast.

Thanks again!

Hi @Nikhil-sinhaa, Thank you for your first time contribution.
From my POV I think this is a good starting. The colors here are currently not the best, your suggestion is better but it's not my decision though.

Anyways, this PR is a feature and it should be rebased to 6.2.
5.4 is no longer accepting features, only bug fixes.

You can also close this PR and open another one against 6.2 if it would be easier for you.
Also, you can add the target branch name in the PR title like this: [6.2] Fix dark mode color contrast.

Thanks again!

You can also close this PR and open another one against 5.4 if it would be easier for you.

@CSGoat0 i guess you meant „against 6.2-dev“

You can also close this PR and open another one against 5.4 if it would be easier for you.

@CSGoat0 i guess you meant „against 6.2-dev“

ahh, my bad thank you.

Anyways, this PR is a feature and it should be rebased to 6.2.

Not necessarily. If it fixes an accessibility (a11y) issue without introducing new functionality or changing html markup, it can be considered a bug fix and so would bebop for 5.4-dev. I think this is the case for this PR, and it is ok for 5.4.

Anyways, this PR is a feature and it should be rebased to 6.2.

Not necessarily. If it fixes an accessibility (a11y) issue without introducing new functionality or changing html markup, it can be considered a bug fix and so would be ok for 5.4-dev. I think this is the case for this PR, and it is ok for 5.4.

I agree that colour alone should not be used to indicate a state. Green=good red=bad are meaningless if you can't see the difference between those colours. But in this case they're pretty meaningless and just used to highlight so I don't see a real problem.

I always ask myself does this change make it better (not perfect, just better) than it was before and for me this pr does that.

Can't quite remember, but I feel like we might have looked at this years ago.

Whilst I agree this PR addresses some contrast issues, the HTML semantics are still incorrect.
The diff display should be using <del> and <ins> tags to represent changes.

From a default visual and accessibility perspective, inserted text will contain an underline, and deleted text will contain a strikethrough, which at a bare minimum, will assist users who suffer from Achromatopsia.

Here's a basic example: https://jsfiddle.net/2vgLzumd/
I've added basic colours for additional presentation.

Can't quite remember, but I feel like we might have looked at this years ago.

Whilst I agree this PR addresses some contrast issues, the HTML semantics are still incorrect.
The diff display should be using <del> and <ins> tags to represent changes.

From a default visual and accessibility perspective, inserted text will contain an underline, and deleted text will contain a strikethrough, which at a bare minimum, will assist users who suffer from Achromatopsia.

Here's a basic example: https://jsfiddle.net/2vgLzumd/
I've added basic colours for additional presentation.

Achromatopsia simulation:

Sighted:

Thanks for the feedback. Good point about accessibility. I'll update the implementation to use <ins> and <del> and push a new commit.

Updated the implementation to use semantic and elements for inserted and deleted content while keeping the visual highlighting.

Updated the implementation to use semantic ins and del elements for inserted and deleted content while keeping the visual highlighting.

I agree with @C-Lodder, it's better with <del> and <ins> (and as a colorblind, visual element helps when you can't see difference between 2 colors).

But color contrast is not correct.

It would be easier to set background color differently depending on the theme mode (light or dark).

In admin Joomla, you can use such kind of code:

const bstheme = document.documentElement.dataset.bsTheme;
const darkMode = (bstheme && bstheme === 'dark');

And then, something like that:

if (part.added) {
    let color = darkMode ? '#fff' : '#000';
    let bgColor = darkMode ? '#266303' : '#d4fcbc';
    element.style.color = color;
    element.style.backgroundColor = bgColor;
}

if (part.removed) {
    let color = darkMode ? '#b3b3b3' : '#555';
    let bgColor = darkMode ? '#8f0000' : '#ffbbbb';
    element.style.color = color;
    element.style.backgroundColor = bgColor;
}

Color contrast above: WCAG AA for removed and AAA for added.

I also agree with @drmenzelit that red and green are "wrong" colours, but they are common usage also on github, but better with it's better with and .

But before patch we have this:

If only the font color in dark mode becomes black, it is WCAG AAA

After patch this

I also agree with @drmenzelit that red and green are "wrong" colours, but they are common usage also on github.
I agree with @C-Lodder, it's better with '' and ''

But before patch we have this:

If only the font color in dark mode becomes black, it is WCAG AAA

After patch this

I also agree with @drmenzelit that red and green are "wrong" colours, but they are common usage also on github.
I agree with @C-Lodder, it's better with <del> and <ins>

But before patch we have this:

If only the font color in dark mode becomes black, it is WCAG AAA

After patch this

It is an a11y bug. A PR should go into 5.4

I also agree with @drmenzelit that red and green are "wrong" colours, but they are common usage also on github.
I agree with @C-Lodder, it's better with <del> and <ins>

But before patch we have this:

If only the font color in dark mode becomes black, it is WCAG AAA

After patch this