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

Usage with app shell? #18

Closed
k-schneider opened this issue Mar 23, 2018 · 14 comments · Fixed by #443
Closed

Usage with app shell? #18

k-schneider opened this issue Mar 23, 2018 · 14 comments · Fixed by #443

Comments

@k-schneider
Copy link

Has anyone used this library with an app shell successfully?

Nothing "breaks" exactly but the icons initially render larger than expected until the Javascript execution kicks in and binds the actual content.

Thanks.

@mlwilkerson
Copy link
Member

Thanks for the question, @k-schneider. I'm not yet familiar with an app shell. Might you make a little repro repo that demonstrates the scenario?

It sounds like the CSS that we inject into the head once icons are rendered through the fontawesome API isn't present initially, thus allowing the SVGs to appear larger at first. But I'm having trouble imagining the scenario whereby this would occur.

@devoto13
Copy link
Collaborator

I remember seeing something like this, when I was investigating #15. It is likely caused by CSS being inserted dynamically by FontAwesome library.

@k-schneider Having a repo with simple instructions, which we can run to debug the problem would definitely help to resolve this quicker.

Generally I think that inserting CSS using Angular component styles instead of base FA library is a good idea. This will improve integration with other Angular tools (like server-side rendering) as well. @mlwilkerson this may be something to consider for FA re-packaging we discussed before. E.g. fontawesome-svg-core won't insert CSS dynamically (as it is now), but instead will let framework packages do this in way natural for every framework.

@mlwilkerson
Copy link
Member

Upon a closer look, there may already be the right mechanisms in place to accomplish that, @devoto13. And I'm working right now on the changes to bring this Angular component up to date with our 5.1 release, which is largely motivated by the packaging concerns you raised. So, this would be the perfect time to tweak the way CSS is loaded for the Angular component.

So, here are the tools we have available to us:

  1. config.autoAddCss allows us to stop the automatic insertion of CSS if that's what we want.
  2. dom.insertCss() allows us to insert the CSS when we want.

(While writing this reply, I noticed a bug where calling insertCss() might have inserted a duplicate style block, but I've put in a fix for that, which I expect to be in the 5.1 release.)

So, I'm still not sure, without a repro, what scenario would cause the icons to look incorrectly formatted until a later loading stage, but we do have the option of calling dom.insertCss() as early in the loading process as we want.

I have a hunch, though, that there's some pre-rendering going on here, (or maybe raw svg file copy-paste?) such that you have <svg> elements already in the DOM before the fontawesome JS library loads at all. The reason I think that is because, the way the library works is that on the first time it renders an icon, it inserts that CSS, unless config.autoAddCss == false. If that's the scenario we're talking about here, then no matter how early we call dom.insertCss() (which is made available by the fontawesome JS library), it's not going to solve the problem if there are already <svg>s in the DOM being displayed before the fontawesome JS loads. The way around that, I think, would have to involve figuring going to the site where the pre-rendering is happening and making sure that, at that time, the CSS is added as well as the pre-rendered SVGs.

@devoto13
Copy link
Collaborator

devoto13 commented Mar 24, 2018

I have made a reproduction for this issue here: https://github.com/devoto13/fa-app-shell-demo. And the problem is indeed the missing FontAwesome's styles. Looks like @angular/platform-server does not include any CSS not inserted through Angular (which is quite expected).

@mlwilkerson you're right, there is a pre-rendering happening to build app shell (see README in the repro). Unfortunately I don't see a way how to use dom.insertCss() with @angular/platform-server. IMO the most clean solution would be to disable CSS insertion using JS and use FontAwesome styles in a separate .css file included using @Component({ styleUrls: [ ... ] }) for fa-icon (and potentially other) component. Do you think it is feasible approach?

@wartab
Copy link

wartab commented Mar 26, 2018

Even if this is probably the only solution that works, it kind of defies the point of this whole library. I have investigated into cleaner approaches after my issue from a month ago, but haven't found anything useful.

@devoto13
Copy link
Collaborator

@wartab Can you elaborate on what defies the point of this library?

IMO The app shell is very clean and straight-forward solution for rendering something before Angular bootstraps. Note that this is different from universal, as rendering of the app shell happens at the build time and does not require a server, so it should work perfectly for your use case with Electron app as well.

Once this issue with missing styles is fixed it should be possible to use FontAwesome icons inside app shell.

@wartab
Copy link

wartab commented Mar 27, 2018

I think app shell is great. I just think that having to go back to manually including the CSS files for prerendering defies the purpose of angular-fontawesome. But as I said, this is nothing you can currently work around to my knowledge.

@DaSchTour
Copy link

I think the bigger issue here is, that font-awesomes JS is tightly coupled to DOM. It would make things a lot easier if the "rendering-engine" could be injected depending on the context. It would just need a small abstraction layer that manages all DOM operations of font-awesome. This would allow the usage of frameworks specific rendering functions to be used and so angular could get awareness of what font awesome is doing. I'm not sure about other frameworks, but I think this could also help there or for server side rendering in general.

@robmadole
Copy link
Member

@DaSchTour the tight coupling is an issue, agreed.

So there are a couple options for this. We are going to run into this with Vue, React, Ember, etc. So let's see if we can solve it in a framework-agnostic way.

We already have a "rendering engine". It's the fontawesome-svg-core. It provides the concept of an "abstract" shape or representation of the icon. However, it does not provide the same interface for the CSS. I think this is where the biggest issues come up for server-side rendering, app shell, CSP, etc.

Proposal 1

(this might be what @devoto13 is proposing)

  1. Disable the auto-insertion of the CSS
  2. Use the @fortawesome/fontawesome-svg-core/styles.css file and integrate that into whatever it needs to hook into (is there a way to inline styles into an app shell?)

Proposal 2

  1. We inline styles on the individual <svg> that is rendered. (This has drawback for CSP)

@juliusstoerrle
Copy link

I think proposal 1 is perfect. Maybe the styles.css file can be optimized a little by removing the webkit prefix (eg. for transform, as it seems not required, checked with caniuse.com)?

In Angular CLI we can just add the script to the apps build configuration and it will automatically be included into the webpack build process. That can potentially even be done with a schematic to ease the setup.

@maxisam
Copy link

maxisam commented Apr 1, 2021

@robmadole Hi how do you disable the auto-insertion of the CSS?

Btw, this issue has been here for 2 years do we figure out the solution?

It is a huge issue for universal in Angular. Icon resized after loaded.

--updated

so I have tried

config.autoAddCss = false;

But the whole css is still in the final build of javascript. (my guess is it can't be tree-shaked )

if I included the css, I am forced to double the size of fontawesome css. The workaround is expensive.

@Peco602
Copy link

Peco602 commented Dec 28, 2021

I still have this issue with Angular 13. Please, could you explain me how to solve it?

@devoto13
Copy link
Collaborator

@Peco602 Please see this comment. You need to add the FA styles file to the global styles in your angular.json.

@Peco602
Copy link

Peco602 commented Dec 28, 2021

Thank you very much. It worked!

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 a pull request may close this issue.

9 participants