Skip to content

Comments

[4.0] Use icon to fix space in Help button#27333

Closed
Quy wants to merge 1 commit intojoomla:4.0-devfrom
Quy:25854-help-button
Closed

[4.0] Use icon to fix space in Help button#27333
Quy wants to merge 1 commit intojoomla:4.0-devfrom
Quy:25854-help-button

Conversation

@Quy
Copy link
Contributor

@Quy Quy commented Dec 21, 2019

Pull Request for Issue #25854 .

Summary of Changes

Change the help button to use icon- instead of fa- to be consistent with the other buttons.

Actions button has the same issue. There isn't an equivalent icon for it so it will be handled in a separate PR.

Testing Instructions

Go to Content > Articles
See space at the bottom of the Help button.

Actual result

62202085-89c3b100-b380-11e9-83f2-f82cb5a37ff9

@brianteeman
Copy link
Contributor

This can't be the correct solution. The entire point of using fontawesome was that it would replace the old icon- fonts and in many/most cases you will see that the icon- fonts are mapped to the fontawesome fonts.

@Quy
Copy link
Contributor Author

Quy commented Dec 21, 2019

Well that is not the case at the moment as icons in the toolbar except for Help and Action are icon-.

@brianteeman
Copy link
Contributor

I am referring to this file
https://github.com/joomla/joomla-cms/blob/4.0-dev/build/media_source/system/scss/_icomoon.scss where most (should be all) of the icomoon fonts are mappes to fontawesome

@Quy
Copy link
Contributor Author

Quy commented Dec 22, 2019

So PR is fine as it and map fa-ellipsis-h to the mentioned file?

@infograf768
Copy link
Member

We have recently found out, that although they are indeed mapped, there are differences in padding.

@brianteeman
Copy link
Contributor

Then we have to fix that in css. Just changing an icon is a bandaid solution

@C-Lodder
Copy link
Member

I'd check that the base styling .fa { ... } and .icon { ... } are the same.
To be honest, fa is still incorrect anyway as it's deprecated.

@brianteeman
Copy link
Contributor

brianteeman commented Dec 22, 2019

I can't replicate this issue using chrome on windows
image

image

but i can on firefox

@Quy
Copy link
Contributor Author

Quy commented Dec 22, 2019

Then all the other icons in the toolbar will have to be changed instead of this one only.

@C-Lodder
Copy link
Member

@Quy The whole of core should be using fa[b|r|s] fa-xxx.
icon-xxx is only mapped for 3rd party extensions to keep B/C

@infograf768
Copy link
Member

My experience of the issue is indeed in Firefox, not Safari for example (Macintosh)

@Quy
Copy link
Contributor Author

Quy commented Jan 27, 2020

Closing. It will be fixed in #27657.

@Quy Quy closed this Jan 27, 2020
@Quy Quy deleted the 25854-help-button branch January 27, 2020 18:21
rdeutz pushed a commit that referenced this pull request Feb 3, 2020
* circle

* ellipsis

* Revert "ellipsis"

This reverts commit 8212d77

* "ellipsis"

* exclamation

* exclamation

* bulk replace

* vue

* sql

* scss changes for .fa {

* adding .fas & .far to .fa, .fab

* ran npm ci

* fixed font size issue

* updated -eye and -eye-slash

* Revert "updated -eye and -eye-slash"

This reverts commit beb3771

* change fa-css-prefix to fas
issue #27333

* quickicons

* Update administrator/templates/atum/scss/_variables.scss

revert as it refers to the icon content itself not the calling class

Co-Authored-By: Quy <quy@fluxbb.org>

* Revert "ran npm ci" to remove package-lock.json

This reverts commit 217f924

* revert _header.scss

* revert conflict

* reapply fas fix

* reapply fas fix for _header.scss

* media folder icons

* media folder icon in toolbar

* open eye

* open eye

* Add missing fas class for quickicons

* Add missing fas and fab classes to inactive tables

* remove .fa and .far

* remove more .fa and .far

* resolve conflict

* accidently removed lines

* try again

Co-authored-by: Quy <quy@fluxbb.org>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
brianteeman pushed a commit to brianteeman/joomla-cms that referenced this pull request Feb 4, 2020
* circle

* ellipsis

* Revert "ellipsis"

This reverts commit 8212d77

* "ellipsis"

* exclamation

* exclamation

* bulk replace

* vue

* sql

* scss changes for .fa {

* adding .fas & .far to .fa, .fab

* ran npm ci

* fixed font size issue

* updated -eye and -eye-slash

* Revert "updated -eye and -eye-slash"

This reverts commit beb3771

* change fa-css-prefix to fas
issue joomla#27333

* quickicons

* Update administrator/templates/atum/scss/_variables.scss

revert as it refers to the icon content itself not the calling class

Co-Authored-By: Quy <quy@fluxbb.org>

* Revert "ran npm ci" to remove package-lock.json

This reverts commit 217f924

* revert _header.scss

* revert conflict

* reapply fas fix

* reapply fas fix for _header.scss

* media folder icons

* media folder icon in toolbar

* open eye

* open eye

* Add missing fas class for quickicons

* Add missing fas and fab classes to inactive tables

* remove .fa and .far

* remove more .fa and .far

* resolve conflict

* accidently removed lines

* try again

Co-authored-by: Quy <quy@fluxbb.org>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants