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

chore(atomic): migrate atomic-icon #4998

Open
wants to merge 58 commits into
base: KIT-4022
Choose a base branch
from
Open

chore(atomic): migrate atomic-icon #4998

wants to merge 58 commits into from

Conversation

y-lakhdar
Copy link
Contributor

@y-lakhdar y-lakhdar commented Feb 20, 2025

Atomic Icon Component Specification

📝 Requirements

The atomic-icon component should display an SVG icon with a 1:1 aspect ratio.

  • Fetches and displays SVG icons from URLs.
  • Supports icons from the Atomic package using the assets:// prefix or any other custom location defined by the user.
  • Renders inline SVG strings.
  • Validates and sanitizes SVG content to prevent XSS attacks.
  • Handles fetch errors gracefully.

♿ Accessibility

  • The component is hidden from assistive technologies using aria-hidden="true".

⚡ Performance & Security

  • Async fetching to avoid UI blocking.
  • Sanitization via DOMPurify to prevent XSS.

⚠️ Risks and Challenges

  • Ensuring the security of fetched and inline SVG content (due to the unsafeSVG method).
  • Handling network errors and providing fallback mechanisms.

✅ Checklist

  • 🧪 The component is unit tested
  • N/A 🧪 The component includes E2E tests (i don't think E2E are relevant to the icon component as all use cases are covered by UTs)
  • ♿ The component complies with the Web Content Accessibility Guidelines.
  • 📦 The Lit component is exported in the appropriate index.ts and lazy-index.ts files.
  • 🎨 CSS parts are documented still accessible.

https://coveord.atlassian.net/browse/KIT-3948

y-lakhdar and others added 22 commits February 6, 2025 10:50
@y-lakhdar y-lakhdar changed the base branch from master to KIT-4010 February 28, 2025 16:05
auto-merge was automatically disabled February 28, 2025 16:05

Merge commits are not allowed on this repository

@y-lakhdar y-lakhdar requested a review from a team as a code owner March 3, 2025 20:13
@y-lakhdar y-lakhdar changed the base branch from KIT-4010 to KIT-4022 March 6, 2025 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants