New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Colorscheme #28788
Fix Colorscheme #28788
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you
Co-Authored-By: Brian Teeman <brian@teeman.net>
My test did not produce the same screenshots as you. The sidebar on the login and admin screens are no longer full height This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28788. |
@brianteeman that's strange because I did not change anything about the sidebar heights at all. Is the issue gone when you revert the patch? Edit: just tested with patchtester without issues. Maybe revert other patches, clear caches and try again? Thank you! |
I am not a fan of this change. They exist because we have no outline and serve as the focus indicator which is fairly universally a blue. For example here on github
|
@brianteeman I only desaturated the box shadow it was very light before - In terms of accessibility it's only slighty recognizeable that it changed - it makes the scheme more harmonic as before. I did not change the focus border btw, if thats your concern, only the light shadow. |
I have tested this item 🔴 unsuccessfully on f0120ab This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28788. |
@faustonenci I used my own colors - your screenshots should not match to mine but have better results like in the posted issue: #28215 - For the muted text color of the joomla Version: did you clear your browser cache ? |
the sidebar was an unrelated issue |
Change the disabled button style in header
@faustonenci can you retest now ? |
I restored all the site and the fatches, I applied the patch again, I only see a problem with the joomla version logo which unlike the other logos uses this style class This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28788. |
@@ -40,6 +40,17 @@ | |||
padding: 15px; | |||
} | |||
|
|||
svg.joomla-logo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've raised this before. You should not be targetting SVG's like this in the template SCSS. Dimensions should be done directly within the logo SVG
height: 4.4rem; | ||
width: 2.4rem; | ||
|
||
path { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above. Put the fill within the SVG
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then I need to create another image for the login view, yes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this was already targetting the login logo, see as the code is in _login.scss
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but the login logo is the same as used at other places, when I change the size inside the svg then I need to create a seperate svg only for login. You agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we exclude this logo code issue in another issue? Because I think there is more stuff to change there, we have different logo-blue, joomla-logo-blue and things flying around, I need to change also behaviour when a custom logo is added...
I will clean this up in a seperate PR ok?
Yes but I actually overwrote it in header, thats why I thought it's a cache issue :-( Anyone else has the same issue? |
I done a PR in your branch: coolcat-creations@e3df4f0 |
@Razzo1987 but you removed text-muted, I think it should be muted when the header is disabled? |
@coolcat-creations Bootsrap 4 text-muted color is "var(--atum-text-light) !important" |
@Razzo1987 yes I added the definition for header right from the start - it's in the _header.scss file. You don't see the changes either? |
I'm not understanding the problem compared to the screen: But the result is different from the request: What is the desired result? |
@Razzo1987 - I have an idea, can you change light text value in your settings to #ffffff and see if it works then? if yes I know what to check again. |
Not muted: #fff instead
instead |
Yes is good, |
Now I understand where all the confusion comes from- the other buttons stay white while they should change to the var(--atum-text-light) - or second possibility the Joomla Logo should be white - I think you are right, it should be all set to white in the header then like the other elements. Sorry I misunderstood the issues :-) |
Thank you @Razzo1987 and @faustonenci now it should be finally fine :-) |
I have tested this item ✅ successfully on 5c207ec This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28788. |
@coolcat-creations :hover :( (see image 3) |
Co-Authored-By: Quy <quy@fluxbb.org>
Co-Authored-By: Quy <quy@fluxbb.org>
@Razzo1987 Thank you for your idea about the colorslider - I honestly consider to remove the hue slider for the dark color at all and want to open a RFC issue for it - the hue slider was formaly there to satisfy the needs for the former dark sidebar and different color shades there. now we actually don't need this anymore. I don't know... Anyway this would be a separate issue too. Please let's go forward with this in small steps :-) This PR does not make everything perfect but moves it forward at least. |
sorry for delay test ✅ |
I have tested this item ✅ successfully on d5825c8 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28788. |
Co-Authored-By: Quy <quy@fluxbb.org>
Last change was CS... @richard67 can I ask for RTC? :-) |
@coolcat-creations Has the discussion with @C-Lodder been resolved? |
@richard67 Regarding the Logo: No but thats a completely new PR / Issue to work on which I will do as next. We need to clean up and rename the used .svg Logos. They are not named properly, they are not used properly and the custom logos are not described good enough so that a user can change it in the way he/she wants. I would open the Issue directly afterwards. Edit: Done: #28826 |
Last change was just CS. I've set back the test restults. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28788. |
Please fix conflicts. |
Thankyou for working on this @coolcat-creations ! Much appreciated! |
Was the change described in issue #28906 intended? |
Pull request for #28215
List of changes:
Testing Instructions
Go into atum template style options and chose different colors in every field (this pr does not fix logo issues!)
Check if the template looks nice
Expected result
If you change the colors you should be happy with the result somehow :-)
Actual result
See issue #28215
Documentation Changes Required
Added Sidebar Link Colour parameter