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

Docs: Improve Font awesome SVG example. #11630

Open
tomgruszowski opened this issue Jun 2, 2018 · 3 comments
Open

Docs: Improve Font awesome SVG example. #11630

tomgruszowski opened this issue Jun 2, 2018 · 3 comments
Labels
area: docs Related to the documentation area: material/icon feature This issue represents a new feature or feature request rather than a bug or bug fix help wanted The team would appreciate a PR from the community to address this issue needs: discussion Further discussion with the team is needed before proceeding P5 The team acknowledges the request but does not plan to address it, it remains open for discussion

Comments

@tomgruszowski
Copy link

tomgruszowski commented Jun 2, 2018

Bug, feature request, or proposal:

Proposal

What is the expected behavior?

Font awesome SVG documentation should be improved.

What is the current behavior?

Ng Material FA docs are limited and don't demonstrate SVG best practices. Font awesome imports of SVGs use their own custom object which breaks down the SVG/XML and represents it as an object, Ng Material expects the complete XML to be added to its registry.

This native SVG usage is preferred because Ng Material has very specific CSS styles that make embedding icons as children of mat-icon element not work as expected, this documentation can be improved so 3rd party icons work seamlessly with material.
i.e. embedding the font awesome icon can be done as:
<button mat-menu-item> <mat-icon><fa-icon [icon]="faGoogle"></fa-icon></mat-icon> <span>Google</span> </button>
... this is the 'default SVG way' if you follow FA docs, it produces an additional FA wrapper element which is undesirable as it also has it's own CSS embedded which does not always play nicely with materials.

It works best as:
<button mat-menu-item > <mat-icon svgIcon="fab:google"></mat-icon> </button>

What is the use-case or motivation for changing an existing behavior?

Seamless FA + ng Material Icons integration example.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Latest

Suggested Doc

Suggestion Example code for using FA+ng Material Icons:

Add tree shakable Imports:
import { faGoogle } from '@fortawesome/free-brands-svg-icons';

`iconService.addSvg(faGoogle); // add icon to library`

HTML usage:
<mat-icon svgIcon="fab:google"></mat-icon> or <mat-icon svgIcon="google"></mat-icon>

Add helper service:
`
@Injectable()
export class IconService {
constructor(private iconRegistry: MatIconRegistry, private sanitizer: DomSanitizer) { }

addSvg(icon: IconDefinition) {
// need to re-assemble the SVG XML
const svg =
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 ${icon.icon[0]} ${icon.icon[1]}"><path d="${icon.icon[4]}" /></svg>; // use template strings

// consider adding a duplicate check here so you don't do this twice
this.iconRegistry.addSvgIconLiteralInNamespace(
icon.prefix, // prefix here is optional, implement as needed
icon.iconName,
this.sanitizer.bypassSecurityTrustHtml(svg)
);
}
}
`

@bogacg
Copy link

bogacg commented Jun 8, 2018

@tomgruszowski wow...it did work and so simple to implement, thanks. This needs to be in the docs.

@jelbourn jelbourn added feature This issue represents a new feature or feature request rather than a bug or bug fix discussion labels Jun 8, 2018
@jelbourn jelbourn removed their assignment Jul 24, 2018
@jelbourn jelbourn added the help wanted The team would appreciate a PR from the community to address this issue label Jul 24, 2018
@mmalerba mmalerba added needs: discussion Further discussion with the team is needed before proceeding and removed discussion labels Mar 3, 2020
@mmalerba mmalerba added the needs triage This issue needs to be triaged by the team label May 20, 2020
@devversion devversion added area: material/icon docs This issue is related to documentation P5 The team acknowledges the request but does not plan to address it, it remains open for discussion and removed needs triage This issue needs to be triaged by the team labels May 28, 2020
@angular-robot
Copy link
Contributor

angular-robot bot commented Feb 1, 2022

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

@angular-robot
Copy link
Contributor

angular-robot bot commented Feb 22, 2022

Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.

We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

You can find more details about the feature request process in our documentation.

@wagnermaciel wagnermaciel self-assigned this Jun 29, 2022
@mmalerba mmalerba added area: docs Related to the documentation and removed docs This issue is related to documentation labels Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: docs Related to the documentation area: material/icon feature This issue represents a new feature or feature request rather than a bug or bug fix help wanted The team would appreciate a PR from the community to address this issue needs: discussion Further discussion with the team is needed before proceeding P5 The team acknowledges the request but does not plan to address it, it remains open for discussion
Projects
None yet
Development

No branches or pull requests

7 participants