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

Externalize Dashicon, Icon, Notice and Tip components imported from @wordpress/components #2037

Merged

Conversation

eason9487
Copy link
Member

@eason9487 eason9487 commented Aug 2, 2023

Changes proposed in this Pull Request:

It's part of #1833

  • Externalize Dashicon, Icon, Notice, and Tip components.
  • Opt-in the @wordpress/primitives package to use the DEWPed one.
    • This is needed due to Icon compatibility. The icon.type === SVG comparison result needs to be true.
    • Reference
  • Extra changes:
    • Fix missing CSS selectors when renaming AppTooltip CSS name in #1901.
    • Remove an unused component DismissibleNotice that was missed to be removed in #1889.

Screenshots:

📷 Dashicon component on the Product Feed page

image

📷 Icon and Notice components on the Get Started page

image

📷 Tip component on the Get Started page

image

📷 The edit icon color was corrected to gray

image

📷 The CSS style of "N products" text wrapped by AppTooltip was corrected

image

Detailed test instructions:

💡 It would be easier to test this PR by making a few changes locally:

Testing:

  1. npm start
  2. Go to the Get Started page.
  3. Check if the Icon, Notice and Tip components are shown correctly.
  4. Go to step 4 of the onboarding flow.
  5. Check if the style of the "N products" text is corrected as the screenshot above.
  6. Go to the Product Feed page.
  7. Check if the Dashicon components are shown correctly.
  8. Check if the color of edit icon on the Product Feed table is corrected to gray.
  9. Search the codebase to check if all these components are imported from 'extracted/@wordpress/components'.

Changelog entry

@eason9487 eason9487 self-assigned this Aug 2, 2023
@github-actions github-actions bot added the changelog: dev Developer-facing only change. label Aug 2, 2023
@eason9487 eason9487 requested a review from a team August 2, 2023 11:39
Copy link
Member

@ianlin ianlin left a comment

Choose a reason for hiding this comment

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

Thanks for the change, all the components are displayed correctly, LGTM.

Base automatically changed from dev/1833-externalize-flex-and-card to dev/externalize-wp-packages August 9, 2023 09:43
@eason9487 eason9487 merged commit ebca4f5 into dev/externalize-wp-packages Aug 9, 2023
4 checks passed
@eason9487 eason9487 deleted the dev/1833-externalize-icons-notice-tip branch August 9, 2023 09:43
@eason9487 eason9487 mentioned this pull request Aug 22, 2023
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: dev Developer-facing only change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants