-
Notifications
You must be signed in to change notification settings - Fork 3
WIP: Include Font Awesome and Material Icons #7
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,12 @@ | ||
| export const iconMap = { | ||
| export const iconMap={ | ||
| md: { | ||
| styleUrl: "https://fonts.googleapis.com/icon?family=Material+Icons", | ||
| classes: "material-icons md-70", | ||
| }, | ||
| fa: { | ||
| styleUrl: | ||
| "https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css", | ||
| classes: "fa fa-70", | ||
| "https://use.fontawesome.com/releases/v5.1.0/css/all.css", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's stick with version 4.7.0 if at all possible, at least until we can demonstrate working on version. The reason for this is status quo production MyUW is using version 4.7.0, so if this also supports 4.7.0, it'll be more interchangable with what we're already running and the icons reference in the current production data. Switching Font Awesome versions is going to entail auditing/updating those icon usages to ensure they're using icon aliases that will work moving forward and might couple adopting this Web Component with making corresponding data changes. So. Not never on the Font Awesome upgrade, but not in the same transaction as flopping over to this Web Component, is my suggestion. |
||
| classes: "fa-70" | ||
| }, | ||
| }; | ||
|
|
||
|
|
@@ -15,12 +15,12 @@ export const iconMap = { | |
| * @param {string|undefined} icon | ||
| * @returns HTMLElement | ||
| */ | ||
| export function createIconElement(iconType, icon = undefined) { | ||
| const node = document.createElement("i"); | ||
| export function createIconElement(iconType, icon=undefined) { | ||
| const node=document.createElement("i"); | ||
| switch (iconType) { | ||
| case "md": { | ||
| node.setAttribute("class", iconMap.md.classes); | ||
| node.innerText = icon; | ||
| node.innerText=icon; | ||
| break; | ||
| } | ||
| case "fa": { | ||
|
|
@@ -37,7 +37,7 @@ export function createIconElement(iconType, icon = undefined) { | |
| * @returns HTMLElement | ||
| */ | ||
| export function createStyleElement(iconType) { | ||
| const node = document.createElement("link"); | ||
| const node=document.createElement("link"); | ||
| node.setAttribute("rel", "stylesheet"); | ||
| node.setAttribute("href", iconMap[iconType].styleUrl); | ||
| return node; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't necessarily need to have an opinion about code style and white space.
I do suggest that if we're going to do automated reformatting of code that's not otherwise being changed,
CONTRIBUTING.md, might even be possible to include tool configuration metadata in this repository.In other contexts some MyUW projects have tried to "adopt Google style". This has some advantages in tooling support, being three words long, applying across a variety of languages, consisting entirely of a reference to external style so there's less surface to internally argue about and customize style, etc.