Skip to content

Conversation

@BPScott
Copy link
Member

@BPScott BPScott commented Sep 6, 2019

WHY are these changes introduced?

Hookifying the whole class is going to be hard, but hookifying the
wrapper component is pretty easy

This should save 2 levels of nesting - the WithAppProvider, and the context consumer

WHAT is this pull request doing?

Remove withAppProvider and a context consumer from ResourceItem

How to 🎩

Check ResourceItem still renders correctly within a ResourceList

@BPScott BPScott force-pushed the resource-item-no-with-app-provider branch 2 times, most recently from 00ec635 to 4873360 Compare September 6, 2019 21:34
@BPScott BPScott requested a review from tmlayton September 6, 2019 21:37
Copy link
Contributor

@perrupa perrupa left a comment

Choose a reason for hiding this comment

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

Looks great! Just had a quick Q about 3rd parties relying on the default export, but looks perfect otherwise!

Hookifying the whole class is going to be hard, but hookifying the
wrapper component is pretty easy

This should save 2 levels of nesting
@BPScott BPScott force-pushed the resource-item-no-with-app-provider branch from 4873360 to 26f4ca6 Compare September 11, 2019 23:00
@BPScott BPScott merged commit 5529591 into master Sep 11, 2019
@BPScott BPScott deleted the resource-item-no-with-app-provider branch September 11, 2019 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants