-
Notifications
You must be signed in to change notification settings - Fork 9
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
icons: enhanced React SVG component support #264
Comments
This commit adds new functionality in the Generator script that will create a new file based on icons in the directory that defines new React specific components to allow for better React DX. On branch dsande/reactIcons/264 Changes to be committed: modified: scripts/generate.js
This commit adds new functionality in the Generator script that will create a new file based on icons in the directory that defines new React specific components to allow for better React DX. On branch dsande/reactIcons/264 Changes to be committed: modified: scripts/generate.js
This commit adds new functionality in the Generator script that will create a new file based on icons in the directory that defines new React specific components to allow for better React DX. On branch dsande/reactIcons/264 Changes to be committed: modified: scripts/generate.js
This commit adds new functionality in the Generator script that will create a new file based on icons in the directory that defines new React specific components to allow for better React DX. On branch dsande/reactIcons/264 Changes to be committed: modified: scripts/generate.js
This commit adds new functionality in the Generator script that will create a new file based on icons in the directory that defines new React specific components to allow for better React DX. On branch dsande/reactIcons/264 Changes to be committed: modified: scripts/generate.js
After reviewing this PR and working with Ashley B on the Seats team to understand their needs a little more I now have a better grasp on what problems this ticket and attached PR address. In short, importing and using SVG files in React is a P.I.T.A. due to hurdles that are unique to React. @blackfalcon's PR demonstrates a method for taking that burden off the consumer teams by generating JS wrappers for each icon that are easier for teams like Seats to import and use in their React code. Since this is not just a simple bug fix, I am moving this ticket back to Triage so that the Auro Team as a whole can review the feature add and make a team decision regarding it's implementation. For my part, I see this as a nice feature add and I think we should move forward with this solution. Nice work and thanks for helping out, @blackfalcon! |
Moving this to blocked while I explore a similar alternative called SVGR. |
Hey @jason-capsule42, I'd like to get your thoughts on using the SVGR-CLI in the workflow. With this PR the README includes SVGR as part of the project's suggested Webpack configuration, which simplifies the process by eliminating the need to update each individual icon. The changes to the generate script only focuses on creating the necessary import statements for React, such as: import { ReactComponent as Instagram } from "..."; From there, the import { Instagram, InformationFilled } from '@alaskaairux/icons/dist/reactComponents.js'; Using the SVGR-CLI will generate an additional set of React-specific JavaScript files like the following: import * as React from "react";
const SvgInstagram = (props) => (
<svg
xmlns="http://www.w3.org/2000/svg"
aria-labelledby="instagram__desc"
className="instagram_svg__ico_squareLarge"
style={{
minWidth: "var(--auro-size-lg, var(--ds-size-300, 1.5rem))",
height: "var(--auro-size-lg, var(--ds-size-300, 1.5rem))",
fill: "currentColor",
}}
viewBox="0 0 24 24"
{...props}
>
<desc>Instagram social media icon.</desc>
<path d="M17 3a4 4 0 0 1 4 4v10a4 4 0 0 1-4 4H7a4 4 0 0 1-4-4V7a4 4 0 0 1 4-4zm0 1.5H7A2.5 2.5 0 0 0 4.5 7v10A2.5 2.5 0 0 0 7 19.5h10a2.5 2.5 0 0 0 2.5-2.5V7A2.5 2.5 0 0 0 17 4.5M12 7a5 5 0 1 1 0 10 5 5 0 0 1 0-10m0 1.5a3.5 3.5 0 1 0 0 7 3.5 3.5 0 0 0 0-7M17 6a1 1 0 1 1 0 2 1 1 0 0 1 0-2" />
</svg>
);
export default SvgInstagram; One consideration is that this step would need to be incorporated after all other icon generation processes are completed, and there may be a need for further optimization to minimize the resulting JS files. Looking forward to hearing your thoughts. |
@blackfalcon thanks for all the followup on this one. @rmenner has been working with me on this one and is going to take over this ticket from me. Ryan, please comment here about the findings we discussed so far and what your next steps are. Thanks! |
My thoughts.
|
Closing this ticket in favor of a documentation guide. Users should be able to use icons with one install and one added typescript file. AlaskaAirlines/AuroDocsSite#272 |
Is your feature request related to a problem? Please describe.
The current documented React solution is no longer valid. The recommended path for using icons directly with a React project require a few manual steps and then this even get's more complicated when using Typescript.
Describe the solution you'd like
All current React use documentation should be removed.
Suggested updates are related to the work required to refactor this project;
One recommendation is to auto-generate a JS file that reads all the icons in the directory and generates lines of code as seen in this example based off a template.
In addition to this auto-generated file, updated documentation needs to be added to the README file that includes the setup and install instructions and examples like the following:
With this markup
Describe alternatives you've considered
Making people do the work themselves.
Additional context
This work was inspired by working with @ashleybiermannalaska to refactor the SeatsUI due to issues discovered with using auro-icon and its CDN dependency in newer mobile experiences. https://github.com/Alaska-ECommerce/SeatsUI/pull/593/files
Exit criteria
This issue can be closed once there is a useable export file added to the project for React users to consume.
Once completed, a new issue should be created to address any additional TS support needed that could be automated and included into the Icons project.
The text was updated successfully, but these errors were encountered: