-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
CustomSelectControlV2: expose legacy wrapper through private APIs #62936
CustomSelectControlV2: expose legacy wrapper through private APIs #62936
Conversation
Flaky tests detected in 33d06ce. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9700511869
|
33d06ce
to
4f1ccb3
Compare
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
4f1ccb3
to
0a556c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Just a minor thought on the naming choice.
@@ -39,6 +40,7 @@ lock( privateApis, { | |||
CompositeItemV2, | |||
CompositeRowV2, | |||
useCompositeStoreV2, | |||
CustomSelectControlV2LegacyWrapper, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming nit: is this really a wrapper though? Since the legacy component isn't really a wrapper as part of a composable component group, but rather a monolithic component, I'd prefer naming this CustomSelectControlV2Legacy
or CustomSelectControlV2LegacyAdapter
since Wrapper
implies a wrapper component in the same way the Tabs
parent component wraps the rest of the subcomponents.
No strong feelings about it, but I feel like others might potentially be confused by that naming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, pushed an update
…rdPress#62936) * CustomSelectControlV2: expose legacy wrapper through private APIs * CHANGELOG * Rename export to `CustomSelectControlV2Legacy` --- Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org>
What?
Part of #55023
Make the
CustomSelectControl
V2 legacy wrapper available via the component package's private APIsWhy?
So that we can start experimenting with using the new V2 legacy wrapper instead of V1 in the editor, as we prepare for swapping V1 implementation with the V2 legacy wrapper implementation
How?
By exporting the component via
lock
ed APIsTesting Instructions
Make sure that it is possible to import the component via the
unlock
function from other packages.