[#33403] - [4.0] Add toolbar.scss in Cassiopeia
- Fixed in Code Base
- 9 May 2021
- Medium
- Build: 4.0-dev
- # 33403
- Diff
- rjharishabh:toolbar
User tests: Successful: Unsuccessful:
Pull Request for Issue #33392.
Summary of Changes
Replicate Atum toolbar in Cassiopeia
Testing Instructions
- Log In to the frontend
- Go to Template Setting in Author Menu module or Visit
http://localhost/joomla-cms/index.php/create-a-post/template-settings - Click
Select - Apply PR
- Do
npm run build:css - Refresh the page and Click
Select - See the difference
Actual result BEFORE applying this Pull Request
subhead noshadow missing CSS
Expected result AFTER applying this Pull Request
Added CSS in subhead noshadow
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End Templates (site) NPM Change |
Sorry I am not at my computer to test. I will do tomorrow.
Sorry I am not at my computer to test. I will do tomorrow.
No problem
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33403.
| Status | Pending | ⇒ | Ready to Commit |
| Labels |
Added:
?
NPM Resource Changed
|
||
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33403.
Thank you for this PR @rjharishabh
TBH I'm not 100% happy with the approach to "hard enforce" things like: https://github.com/joomla/joomla-cms/pull/33403/files#diff-e1218a0d5d75ad354fd21362d37ab7fd8bf7731253e4a6daa09c5bb34a10afd0R14-R17 and have a mix of PX + REM. The frontend template is BS5 anyways, so is there no way to go with the frontend styling?
@bembelimen I want to change PX to REM
but there is no direct relation to converting px to rem, it depends on the default pixel size
I replicate Atum toolbar code to Cassiopeia, then I will change there also
Default is: 1rem == 16px
If you use the same toolbar anyways, you can ofc import it directly instead having two versions...
@bembelimen you can't import it because there are atum only variables
| Status | Ready to Commit | ⇒ | Pending |
Back to pending due to changes requested in comments above.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33403.
@bembelimen you can't import it because there are atum only variables
Yes, there are atum specific variables like atum-bg-dark , atum-link-color, atum-special-color
| Labels |
Added:
?
|
||
@bembelimen why are you asking for the rem to be converted to px? This file was identical to the one in atum except for the color variables. I'm wondering what the logic is for that request.
The link you posted as an example just opens the entire file so its not clear if you really meant the entire file or just some specific parts.
/me confused and can't see anything wrong with the original pr
Sorry my fault, the link was only for the ".row" because that's a default element exactly for adding the margin. So it hurts a bit my guts to remove the margin that way, just wanted to point at it as example for "hard enforce".
The px => rem thing was independent from it another issue. We should not mix it I think but have rem when not a 1px border.
I understand you now. However as its just the same in atum I wouldnt bother to change it.
the px/rem thing is beyond my skillset. Again its just a direct copy from atum and to be honest seeing a value of .313 is just as ugly
to be honest seeing a value of .313 is just as ugly
Same with me
Now I understand @bembelimen @brianteeman
It's my mistake to understand it in a wrong way
| Labels |
Added:
?
Removed: ? |
||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33403.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33403.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-05-09 17:37:52 |
| Closed_By | ⇒ | richard67 | |
| Labels |
Added:
?
Removed: ? |
||
Thanks!
Thanks for merging
I have tested this item✅ successfully on 59b1cce
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33403.