-
Notifications
You must be signed in to change notification settings - Fork 169
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
MWPW-140452 - Icon authoring in milo using the federal repo and individual SVG assets #3184
Conversation
…o be sharepoint authorable
…e condition w/ decorateIcon()
…Preloaded inView icons and defered others till postSectionLoad.
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
Commits
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #3184 +/- ##
==========================================
- Coverage 96.35% 95.93% -0.43%
==========================================
Files 245 175 -70
Lines 56370 48083 -8287
==========================================
- Hits 54316 46129 -8187
+ Misses 2054 1954 -100 ☔ View full report in Codecov by Sentry. |
libs/utils/utils.js
Outdated
const icons = area.querySelectorAll('span.icon'); | ||
if (icons.length) { | ||
const { default: decorateIcons } = await import('../features/icons/icons.js'); | ||
decorateIcons(icons, config); | ||
} |
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.
Moving this is something we wanted to avoid in the initial PR as I had discussed with @saugatmalla and @ryanmparrish as it adds a blocking request rather than having it live in parallel with the other ones. We collected real world data on this and want to avoid sequential blocking requests in front of the LCP chain.
- We load icons
- We wait
- We load federal.js (as import of icons) & the icon after federal finishes, but without blocking anything
- We wait
- We can start loading the blocks
What we want is
We load blocks, icons, federal.js, the icons (strictly needed for LCP) all at the same time.
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.
Okay got it, I moved that back to where it was and added an additional query so we aren't loosing the icon elements in tables.
This is part two of reverted PR: #2986 and makes changes to address issues that were found. I also made some performance updates to the icon decorating functionality.
To streamline icon management across Milo and other consuming sites, a centralized repository,
federal
, along with a directory namedicons/svgs
, will be established. This will provide shared access to a unified set of icons. The transition will maintain the current authoring experience, while also introducing new opportunities for contributors to expand the icon set. The key points of this change are outlined below:Key Points:
Centralized Icon Repository: A new /assets/icons/svgs directory was added to the /federal/ repo, allowing multiple sites (Milo and others) to use the same set of icons. All icons currently available are listed here /icons/icons.json
Consistent Authoring Notation: Authors will continue to use the current notation, such as
:icon-play:
, to insert icons into content. This ensures a seamless transition with no change in authoring experience.Source Change: Icons will no longer be served from the Milo code-bus. Instead, they will be fetched from the federal repository via the content-bus.
Author Contributions: This new system enables authors to contribute and expand the icon set by adding new icons to the centralized repository, a feature that was previously unavailable.
Subsequent Ticket: The Sidekick plugin library will be updated in a subsequent ticket to improve authoring accessibility for that component
Resolves: MWPW-140452
Draft URLs:
Doc URLs:
All consuming sites should be tested for this change.
?milolibs=sartxi-fed-icons
https://main--bacom--adobecom.hlx.page/?milolibs=sartxi-fed-icons