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

Layout: Expose Provider #805

Merged
merged 3 commits into from
Aug 19, 2020
Merged

Layout: Expose Provider #805

merged 3 commits into from
Aug 19, 2020

Conversation

andy-hook
Copy link

@andy-hook andy-hook requested review from bpierre and Evalir August 19, 2020 12:06
bpierre
bpierre approved these changes Aug 19, 2020
View reviewed changes
Copy link
Contributor

@bpierre bpierre left a comment

Choose a reason for hiding this comment

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

LGTM!

src/components/Layout/Layout.js Outdated
Comment on lines 91 to 92
// Can be used to build an alternative Layout component
Layout.__Context = LayoutContext
Copy link
Contributor

@bpierre bpierre Aug 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

If we plan to use it outside of aragonUI, should we make it part of the public API by removing the __?

Copy link
Author

Choose a reason for hiding this comment

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

I was curious about why this exists. What was the original use case? it feels redundant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I completely forgot I added this, I thought it was a new change. It was added in this PR that was made for Apiary, to do the exact same thing we need to do now 😄

Apiary is still using aragonUI 1.0 and this was a private feature, so I think we can just remove it and recommend them to move to LayoutProvider whenever they decide to upgrade aragonUI.

@andy-hook andy-hook merged commit 8277ade into master Aug 19, 2020
@andy-hook andy-hook deleted the layout-seperate-provider branch August 19, 2020 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bpierre bpierre bpierre approved these changes

@Evalir Evalir Awaiting requested review from Evalir

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Layout: Expose provider independently of container
2 participants
@andy-hook @bpierre