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(styles): remove CSS icons #3947

Merged
merged 4 commits into from
Nov 14, 2024

Conversation

alizedebray
Copy link
Contributor

No description provided.

@alizedebray alizedebray requested review from a team as code owners November 8, 2024 16:03
@alizedebray alizedebray linked an issue Nov 8, 2024 that may be closed by this pull request
Copy link

changeset-bot bot commented Nov 8, 2024

🦋 Changeset detected

Latest commit: d0f8e3b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@swisspost/design-system-styles Major
@swisspost/design-system-components-angular-workspace Patch
@swisspost/design-system-components Major
@swisspost/design-system-documentation Patch
@swisspost/internet-header Patch
@swisspost/design-system-intranet-header-workspace Patch
@swisspost/design-system-nextjs-integration Patch
@swisspost/design-system-styles-primeng-workspace Patch
@swisspost/design-system-intranet-header Major
@swisspost/design-system-styles-primeng Major
@swisspost/design-system-components-react Major
@swisspost/design-system-components-angular Major
@swisspost/design-system-intranet-header-showcase Patch
@swisspost/design-system-tokens Major
@swisspost/design-system-icons Major
@swisspost/design-system-migrations Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@alizedebray alizedebray changed the title chore(styles): remore CSS icons chore(styles): remove CSS icons Nov 8, 2024
@swisspost-bot
Copy link
Contributor

swisspost-bot commented Nov 8, 2024

Related Previews

@alizedebray alizedebray changed the base branch from main to 3630-component-validation November 8, 2024 16:03
Copy link
Member

@gfellerph gfellerph left a comment

Choose a reason for hiding this comment

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

Awesome! That's a huge change for a bright future.

@@ -61,9 +61,9 @@

@if (map.has-key(icons.$svg-icon-map, #{$icon-name})) {
$svg-url: map.get(icons.$svg-icon-map, #{$icon-name});
} @else if(map.has-key(icons.$svg-unofficial-icon-map, #{$icon-name})) {
} @else if(map.has-key(icons.$next-icons, #{$icon-name})) {
Copy link
Member

Choose a reason for hiding this comment

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

What are next-icons? Seems like a name that will age badly (what will the next next-icons be called? Next-next-icons?).

Copy link
Contributor Author

@alizedebray alizedebray Nov 11, 2024

Choose a reason for hiding this comment

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

@gfellerph These are PPNL icons, I suppose they will come from the icons package in a near future but I can rename the map. What name would you choose?

Copy link
Member

Choose a reason for hiding this comment

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

@alizedebray sync with @oliverschuerch, I think he called the set UI icons.

Copy link
Contributor

Choose a reason for hiding this comment

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

Rouven defined them as the „UI“ Icons, because they are exclusively made for the UI.
Since each of these icons lifes in a seperate file, contains up to 6 size variants, which are nested in <defs> and <symbol> elements and used through multiple <use> tags combined with dark container query magic defined in the <post-icon> component, I‘d not expect it’s even possible to implement them correctly in SCSS.

@alizedebray If you really want to see how far you can get, I can send you some of the output files already. Or I can open a draft PR (with out the <post-icon> being ready), so you can export them by your self or start the icons preview page, where you’ll see how they work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oliverschuerch we are not talking about the same icon set. What I named $next-icons are not the responsive icons but the 3 notification icons used for the PPNL form validation:
image

I'll rename the map to $notification-icons.

Copy link
Member

@gfellerph gfellerph left a comment

Choose a reason for hiding this comment

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

I'll approve as soon as unit and e2e tests are green ^.-

@alizedebray alizedebray force-pushed the 3868-are-css-icons-still-needed-for-v9 branch from fd62178 to 89592ae Compare November 11, 2024 15:38
@alizedebray alizedebray changed the base branch from 3630-component-validation to main November 11, 2024 15:38
Copy link

sonarcloud bot commented Nov 13, 2024

@alizedebray alizedebray merged commit fa93063 into main Nov 14, 2024
11 checks passed
@alizedebray alizedebray deleted the 3868-are-css-icons-still-needed-for-v9 branch November 14, 2024 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Are CSS icons still needed for v9?
4 participants