Skip to content

[4.0] Safari fix#29624

Merged
wilsonge merged 1 commit intojoomla:4.0-devfrom
brianteeman:install-safari
Jun 17, 2020
Merged

[4.0] Safari fix#29624
wilsonge merged 1 commit intojoomla:4.0-devfrom
brianteeman:install-safari

Conversation

@brianteeman
Copy link
Copy Markdown
Contributor

PR for #29621

apply the safari fix from the admin template #28921 to the installer

@nikosdion @PhilETaylor please test as I don't have access to safari

Dont forget to npm i or node build.js --compile-css

PR for joomla#29621

apply the safari fix from the admin template joomla#28921 to the installer

@nikosdion @PhilETaylor please test as I don't have access to safari
@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Jun 15, 2020
@C-Lodder
Copy link
Copy Markdown
Member

The reboot is already being imported from inside the core BS SCSS file:

@import "../../../media/vendor/bootstrap/scss/bootstrap";

@import "reboot";

@brianteeman
Copy link
Copy Markdown
Contributor Author

It may well be but the fix is NOT in that file.

@C-Lodder
Copy link
Copy Markdown
Member

I know, so you can just add:

th {
  text-align: -webkit-match-parent;
}

Rather than importing the entire reboot.scss again

@brianteeman
Copy link
Copy Markdown
Contributor Author

Rather than importing the entire reboot.scss again

I am not doing that - check the paths

@C-Lodder
Copy link
Copy Markdown
Member

Ah wrong template, sorry.

@dgrammatiko
Copy link
Copy Markdown
Contributor

I am not doing that - check the paths

Actually you are doing exactly that. the line 6:

@import "../../../media/vendor/bootstrap/scss/bootstrap";

Already includes reboot, so you're including the same (-ish) file twice. Either:

  • what @C-Lodder proposed above
  • or unpack @import "../../../media/vendor/bootstrap/scss/bootstrap"; to the individual imports and replace the reboot with the preferred one...

@brianteeman
Copy link
Copy Markdown
Contributor Author

brianteeman commented Jun 15, 2020

please read the pr before commenting!!
even better test it

@dgrammatiko
Copy link
Copy Markdown
Contributor

@brianteeman I'm sorry but the naming is really poor here. The file is not reboot rather an add-on to reboot, so, unless someone really digs into the code it's rather frustrating. You could have named that as reboot-safari-fix or something similar and things would be obvious. Naming that file as a known Bootstrap module and expect people to understand what's inside that file is a bit stressed to say the least...

@brianteeman
Copy link
Copy Markdown
Contributor Author

it is named reboot because it follows the existing naming convention that I did not create.

I can't help it if people like to comment that something is wrong and doesnt work when they dont test it

@wilsonge wilsonge merged commit 820dbad into joomla:4.0-dev Jun 17, 2020
@wilsonge
Copy link
Copy Markdown
Contributor

Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone Jun 17, 2020
@brianteeman
Copy link
Copy Markdown
Contributor Author

Thanks @wilsonge

@brianteeman brianteeman deleted the install-safari branch June 17, 2020 16:16
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
PR for joomla#29621

apply the safari fix from the admin template joomla#28921 to the installer

@nikosdion @PhilETaylor please test as I don't have access to safari
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.

5 participants