refactor(many): extract raw svgs for cross-package use#2543
refactor(many): extract raw svgs for cross-package use#2543HouseOfHawks wants to merge 1 commit into
Conversation
|
|
|
HouseOfHawks
force-pushed
the
TASTY-1731
branch
3 times, most recently
from
68f4ad5 to
c66c76d
Compare
Visual regression report✅ No changes.
Baselines come from the |
|
Can you please add a postfix to the new icons? e.g. |
| * SOFTWARE. | ||
| */ | ||
|
|
||
| import { useState, memo, useCallback, useMemo, useRef } from 'react' |
There was a problem hiding this comment.
As we have discussed, isnt this a bad place for Parchment icons? Parchment devs will look for InstUI components in Storybook, and not visit this site? Also it might be confusing to them, why are we here showing only React icons, why not Angular/WebComponents/Svelte/...?
And its confusing for Instructure users too, why show here icons, that they will never use? I would not place Parchment icons into the Instructure-side InstUI documentation
There was a problem hiding this comment.
Instructure devs will be coming here to look for icons in their Figma designs, which may include Parchment vectors/graphics.
We will point Parchment devs to the upcoming Storybook, which will show the new Instructure icons (new to them) as well as the pre-existing vectors and graphics made available from Parchment - and temporarily have some icon overlap until we dedupe/prune the list to one that is unified.
Remember, the vectors/graphics are product branded and likely to be used in Instructure apps as we integrate our products more - something we are already doing with two Instructure teams.
Since this page is only for Instructure devs, we are only showing React icons. If you recall, the expectation is not to build Angular/WebComponents/Svelte for the Instructure stakeholders as that's not a technical requirement there. That will be handled for Parchment stakeholders only and it will live in the upcoming Storybook page where we will direct Parchment stakeholders.
There was a problem hiding this comment.
hmm OK, I guess its also a better DX because we dont hide some icons, that are accessible via imports.
| export * from './generated/parchment/icons' | ||
| export * from './generated/parchment/graphics' | ||
| export * from './generated/parchment/vectors' |
There was a problem hiding this comment.
Please suffix all icons, so its easily distinguishable on the code side what kind of icons are these e.g. AddDocumentLineParchment
There was a problem hiding this comment.
On vectors/graphics, too? We had discussed adding the suffix on the icons, since there's overlap, but there's no overlap in vectors/graphics.
We can make this adjustment - I only hesitate because this makes some of these names very long and I'm just being empathetic to the dev experience.
There was a problem hiding this comment.
hmm I understand. In this case I would still add some prefix/suffix to these too, now the ui-icons package exports 2000+ graphics, which is super hard to navigate:
*InstUIIcon-- new InstUI icons (1900+)*ParchIcon-- Parchment icons[no logic]-- Parchment graphics (100+)[no logic]-- Parchment vectors (50+)Icon*-- Legacy, deprecated InstUI icons (must have this name for backwards compat, will be deleted in the future)
I think a prefix or a suffix would help all devs to navigate this huge number of exports.
There was a problem hiding this comment.
Thinking through the DX - maybe pivoting on the plan by adding a prefix is a better experience; and also safer via import. When an app dev is intentionally looking for a Parchment Icon and is typing the import, starting with Parch..., it's a very clear direction to Parchment icons/vectors/graphics and most (if not all) InstUIIcons get filtered - save potential edge cases. In the other direction as an Instructure dev is importing the <AccessibilityInstUIIcon>and has typed Acc..., all Parchment icons/vectors/graphics are filtered in the IDE auto-pop.
There was a problem hiding this comment.
Looks like I need the semantics deep merge fix - rebased on pushed anyways, but build is expected to fail until we have #2561
There was a problem hiding this comment.
Odd that the build didn't fail. 🤷
extracted raw svgs into a new ui-icons-svg package and added as a dependency in ui-icons feat(ui-scripts,ui-icons,ui-icons-svg): added parchment assets added parchment icons, vectors and graphics retooled script bundling to support parchment asset categories extended demo app to conditionally render all icons by org extended demo app to conditionally render parchment icons by type refactor(ui-icons-svg): icons cleanup cleaned up now unused directory refactor(ui-scripts): typescript fix fix(github actions): pin chromaui/action to v11 ci(github actions): removed debugging change refactor(ui-scripts): rebase and add suffix Handled a complex rebase and added a ParchIcon suffix to Parchment Icons refactor(ui-scripts): pivot to prefix parchment icons will now prefix icons - no longer used as a suffix
3 participants
This PR:
ui-icons-svgpackage, free of framework dependencies, for use in other icon-generating packages,ui-icons-svg/svg/Parchmentfor allGraphics,IconsandVectorsused in Parchment, Digitary, Scribbles and Quottly applications,ui-iconswhile making new Parchment icons available in this React package (both legacy and beta), and__docs__app so that consumers can now filter icons by organization type and asset type when viewing Parchment assets (demo below)Tech-Spec Outlining the Justification For This Work
Demo:
Screen.Recording.2026-05-06.at.8.46.53.AM.mov
Next iterations (ongoing work):
Currently there is significant asset overlap. Design teams will need to work through both asset libraries and unify that overlap (ie, InstUI's
x.svgand Parchment'sclose.svg). Parchment application developers will need access to this full set of assets initially as teams begin implementing InstUI, but as re-design work becomes available those teams will be pointed to the InstUI icons where applicable. In other cases where assets are Parchment-specific or there is no InstUI counterpart, those Parchment assets will remain.