Replace deprecated icons in EUI components#9375
Conversation
tkajtoch
force-pushed
the
feat/9371-replace-deprecated-icons
branch
from
0ef17d8 to
7daaa92
Compare
tkajtoch
added
the
skip-changelog
|
Some of the VRT updates come from |
|
@tkajtoch some time ago, @JoseLuisGJ suggested a story with all icons (kind of like the "kitchen sink"). Could we possibly add that on this PR?
Regarding this, we discussed with @mgadewoll that some of the VRTs are flaky or unexpected. I was planning to fix that on a separate PR. But no action for this PR, we can still merge these updates. |
There was a problem hiding this comment.
doubt:
This is a very weird change. Is this expected?
There was a problem hiding this comment.
It's not expected, but comes from the sensitivity of our current VRT setup, so I left it as-is 🤷🏻 We really need to do something about these tests and I'm looking into possible solutions
There was a problem hiding this comment.
I'm looking into this ATM so let's sync on 1:1. I agree we should do sth about it but of course this is not the concern for this PR!
There was a problem hiding this comment.
doubt:
This is a rather unexpected change. We can merge it to main to silence it and work off clean slate but just to point it out.
There was a problem hiding this comment.
out-of-scope:
Just noticed that the note by boxesHorizontal says // NOTE: To be deprecated in favor of boxes_vertical``. It should be boxesVertical. Just pointing out.
There was a problem hiding this comment.
Weird that it doesn't highlight the actual iconType value change 😄
|
I see that there are some deprecated icon usages still in documentation website (e.g. |
|
Moving here also this concern I have regarding this PR and it's related GH ticket |
tkajtoch
force-pushed
the
feat/9371-replace-deprecated-icons
branch
from
17c72f4 to
1f78bce
Compare
packages/website/docs/patterns/nested-drag-and-drop/example.tsx
Outdated
Show resolved
Hide resolved
weronikaolejniczak
left a comment
weronikaolejniczak
left a comment
There was a problem hiding this comment.
Thanks for updating the icons in the whole repo! I found only one wrong handler 😄 but I found a bunch of not replaced cases:
cheer → popper
packages/website/docs/components/data-grid/data-grid.mdx:179
editorBold → textBold
packages/website/docs/components/forms/layouts/compressed-forms.mdx:461
editorItalic → textItalic
packages/website/docs/components/forms/layouts/compressed-forms.mdx:467
editorUnderline → textUnderline
packages/website/docs/components/forms/layouts/compressed-forms.mdx:473
editorStrike → textStrike
packages/website/docs/components/forms/layouts/compressed-forms.mdx:479
help (it says NOTE: Might be deprecated later (not recommended in Kibana))
packages/eui/src/components/header/header.stories.tsx:86packages/website/docs/components/layout/header.mdx:561packages/website/docs/components/layout/header.mdx:647
search → magnify
packages/eui/src/components/icon/icon.test.tsxpackages/eui/src/components/form/form_control_layout/_num_icons.test.ts:20packages/website/docs/components/navigation/context-menu.mdx:52packages/website/docs/components/navigation/buttons/split-button.mdx:245
starFilled → starFill
packages/website/docs/components/data-grid/schema-and-columns.mdx:142
starFilledSpace → starFillSpace
packages/website/docs/components/navigation/buttons/button.mdx:990
stopFilled → stopFill
packages/website/docs/components/forms/layouts/controls.mdx:94
It's expected that packages/website/docs/components/display/icons/icon_types.ts mentions icons to be deprecated?
@weronikaolejniczak yes, we're replacing the usages but not removing the actual icons or mentions in this PR. We'll do that separately |
# Conflicts: # packages/eui/src/components/date_picker/auto_refresh/auto_refresh.tsx # packages/eui/src/components/date_picker/super_date_picker/quick_select_popover/quick_select_popover.tsx # packages/eui/src/components/form/form_control_layout/form_control_layout.stories.tsx # Conflicts: # packages/eui/src/components/flyout/flyout_menu.tsx
Some VRT updates come from `main` and are unrelated to changes in this PR.
Co-authored-by: Weronika Olejniczak <32842468+weronikaolejniczak@users.noreply.github.com>
tkajtoch
force-pushed
the
feat/9371-replace-deprecated-icons
branch
from
d4189b3 to
6f40a9d
Compare
|
@weronikaolejniczak I fixed all icons you mentioned above and grepped all source files in all packages to ensure there are no other usages of the deprecated icons. Thank you for a thorough review! |
packages/website/docs/components/data-grid/schema-and-columns.mdx
Outdated
Show resolved
Hide resolved
Co-authored-by: Weronika Olejniczak <32842468+weronikaolejniczak@users.noreply.github.com>
Co-authored-by: Weronika Olejniczak <32842468+weronikaolejniczak@users.noreply.github.com>
💚 Build SucceededHistory
cc @tkajtoch |
💚 Build Succeeded
History
cc @tkajtoch |
|
The CLA check seems to be broken on this PR and shows status Update: admin merge wasn't needed. CLA check updated when I posted this comment. |
4 participants
Summary
Resolves #9371
This PR replaces deprecated icons internally in our components based on the @JoseLuisGJ recommendations in
icon_map.ts.Why are we making this change?
Part of the Iconography M2 initiative. We want to use correct, non-deprecated icons in EUI.
Impact to users
There are no breaking changes in this PR. DOM snapshots of updated components may change. The PR is marked as
skip-changelogas there are no user-facing changesQA
Remove or strikethrough items that do not apply to your PR.
General checklist
Generally speaking, VRT tests cover most (if not all) uses of the updated icons and show no changes, so manual verification is not mandatory. I recommend comparing a few important components modified in this PR: