[MGNLUI-3613] Magnolia page-editor.css conflicts with website css / font-awesome Created: 19/Aug/15  Updated: 15/Apr/16  Resolved: 02/Oct/15

Status: Closed
Project: Magnolia UI
Component/s: page editor
Affects Version/s: 5.4
Fix Version/s: 5.4.3

Type: Bug Priority: Minor
Reporter: Mathias Conradt Assignee: Mikaël Geljić
Resolution: Fixed Votes: 0
Labels: quickwin
Remaining Estimate: 0d
Time Spent: 7.5h
Original Estimate: 3h

Attachments: PNG File close-icon.png     PNG File correct-icons.png     File magnolia-ui-admincentral-5.4.jar     File page-editor.css     PNG File why-important-is-important.png     PNG File wrong-icons.png    
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: Basel 13
Story Points: 1

 Description   

Magnolia's page-editor.css, line 35:

[class^="icon-"], [class*=" icon-"] {
  font-family: MagnoliaIcons !important;

conflicts with website designs that use font-awesome, such as the popular
Unify bootstrap template.

In preview mode, the website icons are swapped with others incorrectly, while the published site shows the icons correctly.

Screenshots attached.

Recommended solution: change the Magnolia CSS

([class^="icon-"], [class*=" icon-"])

to

".mgnlEditorBar [class^="icon-"], .mgnlEditorBar [class*=" icon-"]"

so that it's explictly just affecting the magnolia bar, not all icons on the page.

Wrong icons in preview mode:

Correct icons in published mode:



 Comments   
Comment by Mathias Conradt [ 20/Aug/15 ]

Fixed jar and css file attached.

Comment by Jan Haderka [ 20/Sep/15 ]

Thanks for the patch.

Comment by Federico Grilli [ 30/Sep/15 ]

Hi sang.ngo, is there a particular reason why you didn't prepend the css class names with .mgnlEditorBar as suggested by the reporter?

Comment by Sang Ngo Huu [ 01/Oct/15 ]

As we know, class [class^="icon-"], [class*=" icon-"] come from magnolia-icon.scss which is included most of scss files in Magnolia, not specific for page.

I saw that modifier !important on font-family: MagnoliaIcons; which not allowed css in template of page app override it. And if I prepend .mgnlEditorBar, modifier still works, and it still not solved. This is the rootcause of this problem.

Comment by Federico Grilli [ 01/Oct/15 ]

sang.ngo In the attached image you can see how removing !important for that style prevents from displaying the close icon correctly. I'd propose to go for the fix suggested by the issue reporter and narrow down the change to the page editor styles only.

Comment by Mikaël Geljić [ 02/Oct/15 ]

... with the slight challenge that the font is not a Sass mixin, and most likely cannot be one (entirely) because of the @font-face in there. O:) Then you could do:

.mgnlElement {
  @include magnoliaIcons;
}

Also, Git tells us I introduced this back when I was working on MGNLUI-624; commit message stated:

Also make sure icon classes have the proper font now that icomoon applies it without the :before pseudo class

So I guess there was at least one scenario where icons could get a font-family inherited by more specific selectors, rather than the icon-font defined here. I couldn't confirm that by trying for 5 minutes, but it doesn't mean it doesn't exist. Regardless, isolation with .mgnlElement selector is a way to go .

Comment by Federico Grilli [ 02/Oct/15 ]

I tried using the mixin but with no dice. Even though apparently there was no particular error in Sass compilation output, the mixin did not make it in the final styles file, so no icons were visible. I actually had a weird WARN in eclipse just after Sass compiling but afaict it shouldn't be related to the mixin thingy

WARNING: Error persisting scss cache /Users/fgrilli/devel/workspace/magnolia/CE/magnolia-5.4.x/ce-bundle/magnolia-bundled-webapp/src/main/webapp/VAADIN/themes/admincentral/styles.scss.cache
java.io.FileNotFoundException: /Users/fgrilli/devel/workspace/magnolia/CE/magnolia-5.4.x/ce-bundle/magnolia-bundled-webapp/src/main/webapp/VAADIN/themes/admincentral/styles.scss.cache (No such file or directory)
Comment by Mikaël Geljić [ 02/Oct/15 ]

This happens usually upon first request after startup (after on-the-fly compilation): Vaadin cannot cache the css, but it actually doesn't do any harm.

Comment by Federico Grilli [ 02/Oct/15 ]

A new branch with squashed commits and the fix for the icon alignment issue is at fix/MGNLUI-3613-mixin-sq

Comment by Mikaël Geljić [ 05/Oct/15 ]

Doesn't seem to do the trick (ff).

With the mixin change, selector div.mgnlEditorBar [class*=" icon-"] gets more specific than inherited rules from div.mgnlEditorBarButtons.

The following forces line-height to be inherited, rather than picked from the icon styles (matching the direct element):

page-editor.scss
div.mgnlEditorBar {
  @include magnoliaIcons;
  div.editorIcon {
    line-height: inherit !important;
  }
  ...
}
Comment by Mikaël Geljić [ 07/Oct/15 ]

For future reference, attaching another example showing why the !important tag is important for now.
We will update the comment in the file accordingly:
/* Icons may have their font overridden locally if not !important (e.g. in confirm-dialogs), see MGNLUI-3613. */

Generated at Mon Feb 12 09:08:23 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.