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

Box Control: Add Runtime Check & Conditional Types for presets and presetKey Props #68385

Merged
merged 6 commits into from
Jan 22, 2025

Conversation

im3dabasia
Copy link
Contributor

@im3dabasia im3dabasia commented Dec 30, 2024

Follow up to this Comment: #67688 (comment)

What?

  • Added a runtime warning when only one of presets or presetKey props is defined.
  • Introduced conditional types to ensure presetKey is required if presets is specified.

Why?

  • Prevents potential misconfigurations by ensuring that both props are used correctly together.
  • Enhances type safety and developer experience by enforcing clear prop relationships.

How?

  • Implemented a runtime check for the presets and presetKey props to trigger a warning if only one is defined.
  • Updated TypeScript types to make presetKey non-optional when presets is provided.

Testing Instructions

  1. Define only one of the presets or presetKey props in a component. (For my testing purposes I used the BoxControl storybook)
  2. Ensure a runtime warning is displayed.
  3. Verify that presetKey is required when presets is defined.

Screencast

Screen.Recording.2025-01-03.at.1.18.21.PM.mov

@im3dabasia im3dabasia marked this pull request as ready for review January 3, 2025 07:53
@im3dabasia im3dabasia requested a review from ajitbohra as a code owner January 3, 2025 07:53
Copy link

github-actions bot commented Jan 3, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: im3dabasia <[email protected]>
Co-authored-by: ciampo <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@im3dabasia
Copy link
Contributor Author

Hi @mirka
When you have a moment, Please review my PR.

Thank you 🙇

@mirka mirka requested a review from ciampo January 13, 2025 10:48
@Mamaduka Mamaduka added [Type] Code Quality Issues or PRs that relate to code quality [Package] Components /packages/components labels Jan 13, 2025
@ciampo
Copy link
Contributor

ciampo commented Jan 22, 2025

Could you add a CHANGELOG entry? "Enhancements" sounds like a good section.

@im3dabasia
Copy link
Contributor Author

Sharing a screencast with the latest changes. I have tested it in Storybook's BoxControl component for illustration purposes.

Screen.Recording.2025-01-22.at.9.00.17.PM.mov

@im3dabasia
Copy link
Contributor Author

@ciampo, the pipeline check suggests adding the entry to the Unreleased section. For now, I have added it under the Enhancement section.

Please let me know if any changes are needed or if this can be left as is.

Please make sure your CHANGELOG entry is in the ## Unreleased section

@im3dabasia im3dabasia requested a review from ciampo January 22, 2025 15:58
Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

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

The warning is correct, a new "Enhancements" section should be added under the 'Unreleased' top section. Also, thinking about it, I think that "Internal" could be a better section.

All together, you would need to apply the following changes:

diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
index 6c475147985..6adec34a18b 100644
--- a/packages/components/CHANGELOG.md
+++ b/packages/components/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Internal
+
+-   `BoxControl`: Add runtime check for presets and presetKey ([#68385](https://github.com/WordPress/gutenberg/pull/68385)).
+
 ## 29.2.0 (2025-01-15)
 
 ### Internal
@@ -23,7 +27,6 @@
 -   `Text`: Fix text contrast for dark mode ([#68349](https://github.com/WordPress/gutenberg/pull/68349)).
 -   `Heading`: Revert text contrast fix for dark mode with optimizeReadabilityFor ([#68472](https://github.com/WordPress/gutenberg/pull/68472)).
 -   `Text`: Revert text contrast fix for dark mode with optimizeReadabilityFor ([#68472](https://github.com/WordPress/gutenberg/pull/68472)).
--   `BoxControl`: Add runtime check for presets and presetKey ([#68385](https://github.com/WordPress/gutenberg/pull/68385)).
 
 ### Deprecations
 

@im3dabasia im3dabasia requested a review from ciampo January 22, 2025 16:09
Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Thank you for your work and patience!

@ciampo ciampo enabled auto-merge (squash) January 22, 2025 16:26
@ciampo ciampo merged commit b771bdd into WordPress:trunk Jan 22, 2025
64 checks passed
@github-actions github-actions bot added this to the Gutenberg 20.2 milestone Jan 22, 2025
Gulamdastgir-Momin pushed a commit to Gulamdastgir-Momin/gutenberg that referenced this pull request Jan 23, 2025
…`presetKey` Props (WordPress#68385)

* feat: Improve developer experience for presets

* fix: change Warning error message

* fix: Enforce runtime check for presets, and presetKey

* doc: Add log for runtime check

* doc: Add in unreleased section

---

Co-authored-by: im3dabasia <[email protected]>
Co-authored-by: ciampo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants