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

Support more icons in cod-icon #105

Closed
maxatdetroit opened this issue Oct 11, 2023 · 2 comments · Fixed by #107
Closed

Support more icons in cod-icon #105

maxatdetroit opened this issue Oct 11, 2023 · 2 comments · Fixed by #107
Assignees
Labels
enhancement New feature or request

Comments

@maxatdetroit
Copy link
Member

maxatdetroit commented Oct 11, 2023

Is your feature request related to a problem? Please describe.

I'm trying to create a drupal content type that allows editors to select the type of icon they want to display in a button. However, our current <cod-icon/> custom element only supports a limited set of icons, and the <cod-button/> component does not support children element (e.g. something like <cod-button ...> <span><svg ...></svg></span></cod-button>).

This prevents us from having content editors use the Font Awesome Drupal field to select an FA icon because it may not be supported by the design system (i.e. an editor may select fa-chevron-right but <cod-icon data-icon="fa-chevron-right" is not supported) and embedding the icon as a child of <cod-button> will be ignored.

Describe the solution you'd like

Add additional SVG paths to the design system to support FA icons currently used on detroitmi.gov. If people want to use the design system in Drupal and include an icon in a content type, they should use a custom field list with only the supported icon types in the list (instead of using the 'Font Awesome Icon' field type).

This approach could also make sites using the design system more performance by reducing reliance on external FA libraries.

Expand for details:

This would eventually allow a Drupal install to remove the 'Font Awesome Icon' module entirely if they chose to. Without additional configuration, Font Awesome resources can block rendering up to ~1s depending on the connection. Example of FA render blocking with detroitmi.gov:

Screenshot 2023-10-11 at 4 36 09 PM

Describe alternatives you've considered

Alternatives:

  1. Support children elements in cod-button. E.g. support <cod-button ...> <span><svg ...></svg></span></cod-button>.
  • Pros: makes it easier to customize the button for users.
  • Cons: Allows the design system to be misused (e.g. not styling an icon in the button correctly). The button should be left as a leaf component and not have children provided to it for this reason.
  1. Include the FA library in the design system dependencies. Then allow users to pass any FA shortnames (e.g. fa-chevron-right).
  • Pros: Easy support for all FA icons. Compatible with other FA libraries / frameworks like Drupal 'Font Awesome Icon' module.
  • Cons: Increases size and dependencies of design system package. The FA library can be out of sync with external FA libraries/frameworks like Drupal 'Font Awesome Icon' causing some icon names in one framework to be undefined in another.
@maxatdetroit maxatdetroit added the enhancement New feature or request label Oct 11, 2023
@maxatdetroit maxatdetroit self-assigned this Oct 11, 2023
@maxatdetroit
Copy link
Member Author

@jedgar1mx please have a look and let me know if you agree with the suggested approach here (instead of alternatives) before I code this up. This blocks CityOfDetroit/detroitmi/issues/1096.

@maxatdetroit maxatdetroit changed the title Support more FA icons in cod-icon Support more icons in cod-icon Oct 13, 2023
@maxatdetroit
Copy link
Member Author

Synced with Edgar offline. We agree this is the right approach. That being said, the DS uses bootstrap icons (not FA), so we'll use those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant