User tests: Successful: Unsuccessful:
Pull Request for Issue #45224
Adding missing scss variables :
In dark mode:
btn-secondary-color-hvr
In light mode:
btn-secondary-color-hvr
btn-secondary-bg-hvr
Fallback has also been added in buttons.scss for the hover effect of secondary btn classs
Refer issue #45224
Hover over any button with class btn-secondary ( The Toggle Editor button while creating an article is one such button ) and check that the btn-secondary-color-hvr and btn-secondary-bg-hvr is defined and is being applied via the DevTools ( previously this wasn't the case )
When you hover over a button with class btn-secondary and go to inspect element it says that btn-secondary-color-hvr,
btn-secondary-bg-hvr is not defined.
The above issue should be fixed and btn-secondary-color-hvr, btn-secondary-bg-hvr should apply to the buttons
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 |
Category | ⇒ | Repository NPM Change |
If in any case these variables aren't defined, but i wouldn't be able to name what those cases are, thought i would add it just as a safety measure. I shouldve been more mindful of changing the code , would be happy to revert it back
I am not great with scss but it would appear to me that if the first variable wasnt present then the second one wouldnt be either as they are in the same file
@brianteeman yes, you're right the advantage of having this fallback is if, in the future the variables are moved or something similar to that. Should i remove the fallback ?
@brianteeman yes, you're right the advantage of having this fallback is if, in the future the variables are moved or something similar to that. Should i remove the fallback ?
I leave that decision to someone with better scss knowledge than me
I assume that this needs to be rebased to a different branch as there will be no further 5.2 releases.
Originally posted by @brianteeman in #45229 (comment)
@hiteshm0 Please read the comment by @brianteeman.
You can rebase to 5.3 and we can try to merge it into 5.3.1
Labels |
Added:
NPM Resource Changed
PR-5.2-dev
|
Category | Repository NPM Change | ⇒ | Repository NPM Change Installation Language & Strings Front End Templates (site) |
Title |
|
Labels |
Added:
Language Change
PR-5.3-dev
|
Category | Repository NPM Change Installation Language & Strings Front End Templates (site) | ⇒ | Repository NPM Change |
Labels |
Removed:
Language Change
|
I don't quite understand the testing instructions. With the patch applied I see the change in appearance of secondary buttons in the atum template but I do not see btn-secondary-color-hvr, btn-secondary-bg-hvr in the Inspector. What I see is
.btn-secondary {
--btn-color: #fff;
--btn-bg: #495057;
--btn-border-color: #495057;
--btn-hover-color: #fff;
--btn-hover-bg: rgb(62.05, 68, 73.95);
--btn-hover-border-color: rgb(58.4, 64, 69.6);
--btn-focus-shadow-rgb: 100, 106, 112;
--btn-active-color: #fff;
--btn-active-bg: rgb(58.4, 64, 69.6);
--btn-active-border-color: rgb(54.75, 60, 65.25);
--btn-active-shadow: inset 0 3px 5px rgba(0, 0, 0, 0.125);
--btn-disabled-color: #fff;
--btn-disabled-bg: #495057;
--btn-disabled-border-color: #495057;
}
And it does not change when I add/remove the patch and do npm run build:css
Is the change in button appearance sufficient for this test?
@ceford hello, I apologise if the testing instructions were not proper. Can you please check .btn-secondary:hover in the inspect tool. And is the change in appearance desirable ??
Here is a screenshot of what I see in Firefox Developer Tools. The Toggle Editor button is selected and you can see the compiled .btn-secondary styles. I do not see any btn-secondary-color-hvr and btn-secondary-bg-hvr attributes. Am I using the wrong Inspect tool?
Having explored some more I found the -hvr styles much further down the styles list than I have been before. And absent unless the patch is applied. So the PR works. The change of colour on hover seems rather small, barely noticeable. I wonder if there are any guidelines on that.
I have tested this item ✅ successfully on 0580b7a
The hover contrast in Bootstrap 5.3 buttons is barely evident too so I guess this PR is fine.
why do you need the fallback?