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

Deprecate PanelColor components #10391

Merged

Conversation

chrisvanpatten
Copy link
Member

Fixes #10367.

This PR deprecates wp.components.PanelColor and wp.editor.PanelColor in favour of wp.editor.PanelColorSettings, which is a more robust component for the reasons documented in #10367:

Overall PanelColorSettings seems superior because of its support for multiple color values and for not having the clear button float issue.

It also updates the @wordpress/editor and @wordpress/components changelogs to mention the deprecation.

One note is that deprecating wp.components.PanelColor will mean there's no component to provide a color panel that isn't wrapped inside withColorContext. Users who wish to have a panel with a color palette that uses a separate set of colors than what's defined with add_theme_support('editor-color-palette'); will need to build their own panel component.

As such, it may be desired to deprecate wp.editor.PanelColor, while keeping wp.components.PanelColor and simply fixing the visual bug noted in #10367. However, this would be a use-case that Gutenberg itself does not need/use, so it might not be worth continuing to support.

I've never done a deprecation like this… I think I got everything but would very much appreciate a close look/review 😄

@chrisvanpatten chrisvanpatten added Framework Issues related to broader framework topics, especially as it relates to javascript [Package] Components /packages/components [Package] Editor /packages/editor labels Oct 8, 2018
@chrisvanpatten
Copy link
Member Author

Need a little direction on the failing unit test. Is there a way to tell the test to expect the deprecation warning?

@chrisvanpatten chrisvanpatten requested a review from a team October 8, 2018 13:24
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

Hi @chrisvanpatten thank you for creating a PR with this deprecations. I agree that this component should be deprecated so we are not shipping unused code on 5.0 and we don't have multiple UI's for color picking.
Unfortanelety our new component does not completely replace what the old components offered. I will look into adding the missing functionality to the new component so we can merge this PR 👍
Thank you for your contribution.

@talldan
Copy link
Contributor

talldan commented Oct 10, 2018

Thanks for picking this up @chrisvanpatten!

I think to resolve the test failure it'll be something along the lines of adding expect( console ).toHaveWarnedWith( // ... ), as has been done here:
https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/text-columns/test/index.js#L12

@chrisvanpatten
Copy link
Member Author

This should pass the failing test now, but I think is likely to miss the 4.0 milestone, so the deprecation version may need to be updated.

@chrisvanpatten
Copy link
Member Author

Updated the version numbers with the assumption it can land in 4.1.

@chrisvanpatten chrisvanpatten added this to the 4.1 - UI freeze milestone Oct 17, 2018
Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

I think now that #10457 has been merged, this is good to go. Thanks again for picking this up @chrisvanpatten!

@chrisvanpatten chrisvanpatten merged commit f9efc9f into WordPress:master Oct 18, 2018
@chrisvanpatten
Copy link
Member Author

Thanks @talldan! 🚢


### Deprecation

- `wp.components.PanelColor` has been deprecated in favor of `wp.editor.PanelColorSettings`.
Copy link
Member

Choose a reason for hiding this comment

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

In the context of packages, we should avoid references to WordPress-specific globals when possible (at least for changelog). Console messaging is trickier since we can't yet customize for the WordPress context, but it's a hope there we could.

See: #9753

antpb pushed a commit to antpb/gutenberg that referenced this pull request Oct 26, 2018
* Deprecate PanelColor components

* Update deprecation version numbers

* Fix a test broken during the rebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Package] Components /packages/components [Package] Editor /packages/editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maybe deprecate @wordpress/editor PanelColor?
4 participants