[#44176] - [5.2] atum light mode - Fix modal window link color
- Fixed in Code Base
- 7 Nov 2024
- Medium
- Build: 5.2-dev
- # 44176
- Diff
- RickR2H:fix-link-color
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
With the browser in dark modus, In the backend (Atum template) the links have the wrong color when the template is toggled to the light modus. The reason is that in the component.php only the dark color scheme colors are loaded if the browser is is dark mode. My fix will make sure that the correct colors are used, depending or the color scheme selected.
In dark modus the default hover color in the modal is defaulted to white, which is not correct. This PR fixes also the hover color which is now set to a lighter shade of the default link color.
Testing Instructions
- Set the browser you are using in dark mode.
- In the backend from Joomla select from the User Menu in the top right the link: Light/Dark Mode, and switch the template to light modus.
- Open a modal. This can be done by opening an article and selecting the Created By button in the Publishing tab.
- The link color has a light blue color which is not accessible.
- Switch Joomla to Dark mode.
- The link hover color is white.
Apply the PR
- Switch to light modus.
- The link color is now the correct color and readable.
- Switch to dark mode.
- The link hover color is now light blue.
Actual result BEFORE applying this Pull Request
The link is too light which is a accessibility issue.
In dark mode the hover color white
Expected result AFTER applying this Pull Request
The link has the correct color.
In dark mode the hover color light blue
Link to documentations
Please select:
-
Documentation link for docs.joomla.org:
-
No documentation changes for docs.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration Templates (admin) |
| Labels |
Added:
a11y
Backend Template
Dark Mode
PR-5.2-dev
|
||
@RickR2H Adding the // phpcs:disable PSR1.Files.SideEffects hasn't helped. I think the issue is not the defined('_JEXEC') or die; but the function definition here: https://github.com/joomla/joomla-cms/pull/44176/files#diff-3538b524e6cf4f442f5ca0631475c0ef2b867edbcb23a24b5deb2248c428e252R35-R42 . Maybe it needs to wrap that with // phpcs:disable PSR1.Files.SideEffects and // phpcs:enable PSR1.Files.SideEffects?
I‘ve restored the previous human test result as the changes after that were only hints for PHPCS.
| Title |
|
||||||
@dgrammatiko @richard67 I can't get drone to behave... Any tips?
@dgrammatiko @richard67 I can't get drone to behave... Any tips?
With the previous version with my proposal it was ok. So I suggest to wrap the method again into the // phpcs:disable PSR1.Files.SideEffects and // phpcs:enable PSR1.Files.SideEffects.
I have tested this item ✅ successfully on c21bf32
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44176.
I have tested this item ✅ successfully on c21bf32
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44176.
Why use PHP to manipulate CSS colours, when this can be acheived in native CSS with 1 line of code?
Why use PHP to manipulate CSS colours, when this can be acheived in native CSS with 1 line of code?
Because BS is using a css var of type number, number, number :
https://github.com/twbs/bootstrap/blob/fee9dc2438736a02405412e10ea8445dd9aeac1e/dist/css/bootstrap.css#L40-L47
I've restored the previous human test results in the issue tracker as the commits which have invalidated the test count were only related to making PHPCS happy. In addition I have tested that the PR still works with the last change to an anonymous function.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44176.
Could be a nice option to replace the current functions. More clean! For now the the colors nor loading issues is fixed. This is a nice option to add with a new PR. Some logic is used in the index.php
July 2024 might be too soon:
https://caniuse.com/css-relative-colors
Anyways if the maintainers are ok with it, go for it, it's not my call. I just pointed that the BS requires a weird value and that the relative color from is a new addition 🤷♂️
@RickR2H To be honest, to me the hover colour seems to be inconsistent now when comparing dark mode and light mode.
In light mode it is still black with your PR, so when changing to dark mode I would expect it to be white.
Or vice versa, when coming from dark mode where it is light blue with your PR, and changing to light mode, I would expect it to be some other kind of blue but not black.
But maybe that's just me.
I guess there will be a better solution. I have no clue why the values have to calculated. We can make then fixed variables which is a better option. Probably is has to do with the backend colors...
I have no clue why the values have to calculated.
The blame here goes to Bootstrap, they store the rgb values as a coma separated val: --bs-dark-rgb: 33, 37, 41; https://github.com/twbs/bootstrap/blob/fee9dc2438736a02405412e10ea8445dd9aeac1e/dist/css/bootstrap.css#L40-L47
Probably is has to do with the backend colors...
Nope, search the bs css and you'll see how these values are used
I guess there will be a better solution.
What Charlie suggested but the browser support is only few months...
This is consistent with index.php so I'd merge this if it fixes the bug - then at least if we ever change to @C-Lodder 's way in the future (j6 or whatever) then we can do it consistently. Only thing I'd suggest is that we have this the error_login.php and error_full.php too so we have the same set of css variables in all 3 files.
| Labels |
Added:
RTC
|
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2024-11-07 08:30:46 |
| Closed_By | ⇒ | pe7er |
I have tested this item ✅ successfully on 6150056
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44176.