Skip to content
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

[4.0] Cassiopeia variables #32467

Merged
merged 2 commits into from
Feb 24, 2021
Merged

[4.0] Cassiopeia variables #32467

merged 2 commits into from
Feb 24, 2021

Conversation

brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Feb 20, 2021

This PR cleans up the scss variables being used in cassiopeia - the less we change the easier it is to maintain when upstream changes and we dont miss out on features such as rfs and xxl

  • grid breakpoints - removed no reason to override
  • grid containers - removed no reason to override
  • font-sizes - removed so that we can have responsive fonts sizes which are a key feature of bootstrap 5
  • tables - removed two variables that are identical to upstream. had to keep one of them as it is used in this file
  • list-group - removed as identical to upstream

z-index also needs to be reviewed but thats for another PR as is doing the same review for atum.
all the other cassiopeia overrides need to be checked as well to make sure they are not overriding things that it doesnt need to. This is just a start that I hope some people with more frontend skills can continue in another pr

This PR cleans up the scss variables being used in cassiopeia

grid breakpoints - removed no reason to override
grid containers -  removed no reason to override
@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Feb 20, 2021
@richard67
Copy link
Member

Testing instructions are "Check that the site looks as well as before"?

@brianteeman
Copy link
Contributor Author

Check that it compiles and check the typography page of the sample data

@brianteeman
Copy link
Contributor Author

Someone with better frontned skills than me needs to ensure that the changes from em back to the bootstrap grid and font sizes are the same.

@richard67
Copy link
Member

@drmenzelit Could you have a look on it? And eventually also close your PR #32446 in favour of this and possible future fixes for removing overrides of BS SCSS in Cassiopeia? Thanks in advance. I know it was not your idea to override all the BS stuff in Cassiopeia.

@drmenzelit
Copy link
Contributor

The changes from Brian are fine, I don't see changes in the layout. But in my PR were 2 changes that we can need:
templates/cassiopeia/scss/blocks/_layout.scss and
templates/cassiopeia/scss/blocks/_css-grid.scss
Maybe can my PR be merged first and then this one. Brian only changes variables.scss

@richard67
Copy link
Member

richard67 commented Feb 21, 2021

The changes from Brian are fine, I don't see changes in the layout. But in my PR were 2 changes that we can need:
templates/cassiopeia/scss/blocks/_layout.scss and
templates/cassiopeia/scss/blocks/_css-grid.scss
Maybe can my PR be merged first and then this one. Brian only changes variables.scss

@drmenzelit I've merged your PR and than have solved the conflict in _variables.scss here resulting from that merge. I hope the latter will help with @brianteeman not biting me.

@brianteeman
Copy link
Contributor Author

@richard67 in future I would prefer it if you didnt merge anything into my pr as it breaks my workflow

@richard67
Copy link
Member

@brianteeman Ok, noticed. Just thought it would help.

@drmenzelit
Copy link
Contributor

I have tested this item ✅ successfully on 6770f52


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32467.

@ceford
Copy link
Contributor

ceford commented Feb 22, 2021

I have tested this item ✅ successfully on 6770f52

Compiles successfully and I see the heading font sizes change - a tad larger with the patch applied. Looks OK.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32467.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32467.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 22, 2021
@richard67 richard67 added this to the Joomla 4.0 milestone Feb 22, 2021
@laoneo laoneo merged commit d7e189c into joomla:4.0-dev Feb 24, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 24, 2021
@laoneo
Copy link
Member

laoneo commented Feb 24, 2021

Thanks!

@brianteeman
Copy link
Contributor Author

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants