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

MWPW-140452 - Icon authoring in milo using the federal repo and individual SVG assets #3259

Merged
merged 25 commits into from
Dec 9, 2024

Conversation

ryanmparrish
Copy link
Contributor

This initial PR was reverted due to some issues found w/ tooltips in the table block. re-submitting as this has been resolved.


To streamline icon management across Milo and other consuming sites, a centralized repository, federal, along with a directory named icons/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 - see MWPW-159581

Resolves: MWPW-140452

Draft URLs:

Before: https://main--milo--adobecom.hlx.page/drafts/rparrish/icon/icons-federal?martech=off
After: https://rparrish-fed-icons--milo--adobecom.hlx.page/drafts/rparrish/icon/icons-federal?martech=off

Doc URLs:

Before: https://main--milo--adobecom.hlx.page/docs/library/kitchen-sink/?martech=off
After: https://rparrish-fed-icons--milo--adobecom.hlx.page/docs/library/kitchen-sink/?martech=off

URLs where the tooltip / table issue was found:
https://main--dc--adobecom.hlx.page/acrobat/pricing?milolibs=rparrish-fed-icons&martech=off

⚠️ All consuming sites should be tested for this change. ?milolibs=rparrish-fed-icons
https://main--bacom--adobecom.hlx.page/?milolibs=rparrish-fed-icons

@ryanmparrish ryanmparrish added high-impact Any PR that may affect consumers needs-verification PR requires E2E testing by a reviewer labels Nov 25, 2024
Copy link
Contributor

aem-code-sync bot commented Nov 25, 2024

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

Copy link
Contributor

aem-code-sync bot commented Nov 25, 2024

Copy link

codecov bot commented Nov 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.39%. Comparing base (afe9047) to head (d31368a).
Report is 110 commits behind head on stage.

Additional details and impacted files
@@           Coverage Diff           @@
##            stage    #3259   +/-   ##
=======================================
  Coverage   96.39%   96.39%           
=======================================
  Files         245      245           
  Lines       56746    56835   +89     
=======================================
+ Hits        54698    54787   +89     
  Misses       2048     2048           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR.

@SilviuLCF SilviuLCF self-requested a review December 3, 2024 14:54
@SilviuLCF SilviuLCF added Ready for Stage and removed needs-verification PR requires E2E testing by a reviewer labels Dec 3, 2024
@overmyheadandbody
Copy link
Contributor

Rebased with current stage to get the checks green

@milo-pr-merge
Copy link
Contributor

milo-pr-merge bot commented Dec 4, 2024

Skipped 3259: "MWPW-140452 - Icon authoring in milo using the federal repo and individual SVG assets" due to file "libs/blocks/table/table.css" overlap. Merging will be attempted in the next batch

Copy link

@SilviuLCF SilviuLCF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified

@milo-pr-merge
Copy link
Contributor

milo-pr-merge bot commented Dec 5, 2024

Skipped 3259: "MWPW-140452 - Icon authoring in milo using the federal repo and individual SVG assets" due to file "libs/utils/utils.js" overlap. Merging will be attempted in the next batch

@milo-pr-merge milo-pr-merge bot merged commit 81a5770 into stage Dec 9, 2024
14 checks passed
@milo-pr-merge milo-pr-merge bot deleted the rparrish/fed-icons branch December 9, 2024 09:19
@milo-pr-merge milo-pr-merge bot mentioned this pull request Dec 9, 2024
mokimo added a commit that referenced this pull request Dec 11, 2024
mokimo added a commit that referenced this pull request Dec 11, 2024
#3357)

Revert "MWPW-140452 - Icon authoring in milo using the federal repo and individual SVG assets (#3259)"

This reverts commit 81a5770.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-impact Any PR that may affect consumers Ready for Stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants