;) you could have just asked for changes in my pr. that is the normal way to work ikn a collaborative environment

not a fan of this approach to changing a variable

Sorry @brianteeman I just checked and see I missed to check in my review last week.

I can close here if you would reopen your PR.

not a fan of this approach to changing a variable

I'm afraid I don't quite understand. If we don't use the css variables we declared for this purpose then we could actually do without it completely and remove it.

I dont see anywhere else with this is done. everywhere else it is similar to

box-shadow: var(--atum-box-shadow);

But the idea of having a css variable instead of declaring the values directly, is that I can change the value of the variable (globally or only in a specific context) without having to reassign it to the element.

https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties

yes - exactly my point. The way you have done it makes it no different to declaring the values directly.

I think I struggle a little bit with your explanation

Here we have declared the class sidebar-wrapper should use the variable, that has nothing to do with my PR

And at the moment here we ignore the fact that we actually have a variable for it and do not change the value of the variable in dark mode but set the box-shadow explicitly to none.

An alternative would be

@if $enable-dark-mode {
  @include color-mode(dark) {
    .sidebar-wrapper {
      ...
      box-shadow: 1px 0 0 0 #ffffff0d;
      ....
}

But I don't see the benefit.
For the sidebar-wrapper I see no need to have the box-shadow completely around the wrapper, only on the left side to keep contrast for the end of the sidebar, so I have to modify the value to use the already declared variable for sidebar-wrapper.

Is there something I am missing or I can't see?

never mind - i am not good at explaining this

If you define it as a variable then it is easy to customise. What you have done is no different to box-shadow: 1px 0 0 0 #ffffff0d;

I would have created a new variable --atum-sidebar-shadow in _variables.scss and then used box-shadow: var(--atum-sidebar-shadow)

plus you would then be able to correctly have a shadow in an rtl template where the value of the variable would be different.

_variables.scss
--atum-sidebar-shadow: box-shadow: 1px 0 0 0 #ffffff0d;

_variables_rtl.scss
--atum-sidebar-shadow: box-shadow: 0 0 0 1px #ffffff0d;

plus you would then be able to correctly have a shadow in an rtl template where the value of the variable would be different.

_variables.scss --atum-sidebar-shadow: box-shadow: 1px 0 0 0 #ffffff0d;

_variables_rtl.scss --atum-sidebar-shadow: box-shadow: 0 0 0 1px #ffffff0d;

I did not understand why there should be a different value for rtl.

Based on https://developer.mozilla.org/en-US/docs/Web/CSS/box-shadow#syntax
box-shadow: <offset-x> <offset-y> <blur-radius> <spread-radius> <color>

Reason why I propose LadySolveig#2 for cpanels:
box-shadow: 0 0 0 1px rgba(255, 255, 255, .1);

I may have the syntax wrong - I was doing it from memory

However if you are setting a box shadow to appear on the right hand side of the sidebar then obviously on an RTL display you would need to set the box shadow to appear on the left hand side of the sidebar

This pull request has been automatically rebased to 5.2-dev.

Could you please fix the conflict?

This pull request has been automatically rebased to 5.3-dev.

This pull request has been automatically rebased to 5.4-dev.

I have tested this item ✅ successfully on ed12423

Tested with JBT

• Seen slight fidgeting between light/dark mode before PR
• The full package from PR grafted, and switching between light and dark mode no longer changes the text positions

I have tested this item ✅ successfully on ed12423

I have tested this item ✅ successfully on ed12423

I have tested this successfully! Thanks @LadySolveig

RTC

Thanks @LadySolveig for this PR, and thanks @muhme , @ThomasFinnern and @exlemor for testing.