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.

Sign up for GitHub

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

Jump to bottom

Disabled: Add documentation for 'Disabled.Context' #39627

Merged
merged 5 commits into from
Mar 23, 2022
Merged

Disabled: Add documentation for 'Disabled.Context' #39627

merged 5 commits into from
Mar 23, 2022

Conversation

Mamaduka
Copy link
Member

What?

PR adds documentation for Disabled.Context, introduced in #31134.

Why?

Reading context with useContext should be preferred when possible.

How?

PR updates documentation and adds notes about the preferred method.

@Mamaduka Mamaduka requested a review from ajitbohra as a code owner March 22, 2022 07:31
@Mamaduka Mamaduka self-assigned this Mar 22, 2022
@Mamaduka Mamaduka added [Type] Developer Documentation Documentation for developers [Package] Components /packages/components labels Mar 22, 2022
@Mamaduka Mamaduka requested review from gziolo and ciampo March 22, 2022 07:32
@ciampo ciampo requested a review from mirka March 22, 2022 09:32
ciampo
ciampo reviewed Mar 22, 2022
View reviewed changes
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.

Since context is a standard React API (and there's even a link to the docs), I wonder if we should just keep the hook version of the example and remove the "legacy" version.

@Mamaduka
Copy link
Member Author

@ciampo I was thinking the same. The new React docs also recommend using the useContext hook over Context.Consumer.

Happy to update the readme if you're okay with that.

@ciampo
Copy link
Contributor

ciampo commented Mar 22, 2022

Happy to update the readme if you're okay with that.

Let's do that, thank you!

@Mamaduka
Copy link
Member Author

Updated.

ciampo
ciampo approved these changes Mar 23, 2022
View reviewed changes
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 the patience in addressing the feedback! I really appreciate your help in contributing to the components package

@Mamaduka
Copy link
Member Author

Thanks for the reviews, @ciampo.

@Mamaduka Mamaduka merged commit 1440426 into trunk Mar 23, 2022
@Mamaduka Mamaduka deleted the docs/disabled-context branch March 23, 2022 13:53
@github-actions github-actions bot added this to the Gutenberg 13.0 milestone Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ciampo ciampo ciampo approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@gziolo gziolo Awaiting requested review from gziolo

@mirka mirka Awaiting requested review from mirka

Assignees

@Mamaduka Mamaduka

Labels
[Package] Components /packages/components [Type] Developer Documentation Documentation for developers
Projects
None yet
Milestone
Gutenberg 13.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Mamaduka @ciampo