Skip to content

Cassopeia rtl update#36040

Merged
wilsonge merged 5 commits intojoomla:4.0-devfrom
brianteeman:cassopeia-rtl
Nov 28, 2021
Merged

Cassopeia rtl update#36040
wilsonge merged 5 commits intojoomla:4.0-devfrom
brianteeman:cassopeia-rtl

Conversation

@brianteeman
Copy link
Copy Markdown
Contributor

@brianteeman brianteeman commented Nov 14, 2021

remove skipto override as its fixed upstream
remove metis-toggler as its a duplicate
change metis-toggler to use logical operators
change navbar-brand to use logical operators
change jmodedit to use logical operators

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Nov 14, 2021
@khu5h1
Copy link
Copy Markdown
Contributor

khu5h1 commented Nov 14, 2021

I have tested this item ✅ successfully on bacf8ff


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

1 similar comment
@Shubhamverma2796
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on bacf8ff


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

@HLeithner
Copy link
Copy Markdown
Member

If I'm correct we don't use inset-inline-end till now, is it save for us to use it?

@brianteeman
Copy link
Copy Markdown
Contributor Author

Perfectly safe to use https://caniuse.com/?search=inset-inline-end

@richard67
Copy link
Copy Markdown
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 14, 2021
@pritam825
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on bacf8ff


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

@infograf768
Copy link
Copy Markdown
Member

Screen Shot 2021-11-16 at 08 47 53

@HLeithner
Copy link
Copy Markdown
Member

I have seen this too JM, and I think it breaks our policy CURRENT - 2 (also current depends of the time of the major or minor release, but I'm not sure on this topic).

based on this information all PRs including 'inset-inline-end' is too early for us but have to be discussed by production

@brianteeman
Copy link
Copy Markdown
Contributor Author

i thought it was changed to current -1

@HLeithner
Copy link
Copy Markdown
Member

#PROD2019/005 - Change minimum version requirements for all browsers to n-2 and drop support for Internet Explorer. (per 19/05/16)

was the last motion I found... should be moved to the development strategy

@C-Lodder
Copy link
Copy Markdown
Member

@HLeithner Your package.json states otherwise:

"browserslist": [
    "last 1 version",
    "not ie < 11"
],

@brianteeman
Copy link
Copy Markdown
Contributor Author

and before that it was in settings.json where it was merged 5july 2018

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Nov 16, 2021

FYI, concerning Safari versions: 14.1 was an update for Mac OS Big Sur, i.e. all former versions of MacOS using Safari 14 or lower will not be compatible. Mac OS Monterey has Safari 15.

@HLeithner
Copy link
Copy Markdown
Member

@HLeithner Your package.json states otherwise:

"browserslist": [
    "last 1 version",
    "not ie < 11"
],

then the package.json is wrong and has to be fixed

@C-Lodder
Copy link
Copy Markdown
Member

lol, after 3 and a half years?
Sounds like the motion was incorrectly written without prior research.

@HLeithner
Copy link
Copy Markdown
Member

lol, after 3 and a half years? Sounds like the motion was incorrectly written without prior research.

no idea why it's funny when an error happens. we have 15 year old bugs.

@brianteeman
Copy link
Copy Markdown
Contributor Author

I leave it to those far more important than me to decide. note that the settings are used to determine if autoprefix etc needs to do something to ensure old browser support

@brianteeman
Copy link
Copy Markdown
Contributor Author

Changing to last 2 version will result in a massive amount of changes to the css generated by the autoprefixer. As a very quick example we will see the following changes in build_incomplete.html

image

@richard67 richard67 added the RMDQ ReleaseManagerDecisionQueue label Nov 18, 2021
@richard67
Copy link
Copy Markdown
Member

Setting RLDQ label so release leads can decide about usage of inset-inline regarding our browser support policy.

@brianteeman
Copy link
Copy Markdown
Contributor Author

Further information. Please remember that we have been wrorking and testing with last 1 version for 3.5 years. If you rebuild the current css with last 2 version you will see how much is different - about 9% bigger. (these tests are without by pull requests as they're not merged)

CSS Line Count

Template last 1 version last 2 version
Atum 14516 15884
Atum RTL 14807 16145
Cassiopeia 13552 14845
Cassiopeia RTL 13829 15122

@wilsonge
Copy link
Copy Markdown
Contributor

wilsonge commented Nov 28, 2021

FYI, concerning Safari versions: 14.1 was an update for Mac OS Big Sur, i.e. all former versions of MacOS using Safari 14 or lower will not be compatible. Mac OS Monterey has Safari 15.

So based on this high sierra is the highest OSX version that is affected by this but already had no security support when Joomla 4.0.0 came out (security support ended december 2020). Given our CSS is already bugged in terms of giving support for this version as well I don't think it's a big deal merging these Pull Requests at this moment in time. Especially given the big savings we get in maintainability.

@wilsonge wilsonge merged commit c30286b into joomla:4.0-dev Nov 28, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 28, 2021
@wilsonge wilsonge added this to the Joomla 4.0.5 milestone Nov 28, 2021
@richard67
Copy link
Copy Markdown
Member

@wilsonge Thanks for this decision which I fully support.

@brianteeman
Copy link
Copy Markdown
Contributor Author

Thanks for making a sensible decision.

@brianteeman brianteeman deleted the cassopeia-rtl branch November 28, 2021 17:44
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 RMDQ ReleaseManagerDecisionQueue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants