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

DRY Explore containers #1999

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

kueda
Copy link
Member

@kueda kueda commented Aug 23, 2024

RootExploreContainer and ExploreContainer were basically duplicates with minor tweaks, which was frustrating when you needed to change something in both, and as a result they were starting to drift. This PR unifies all of the logic they share into ExploreContainer and separates the ways in which they differ into RootExploreContainer and StackedExploreContainer.

DRY up Explore container logic so most of it is just in one place, and just
the parts that differ between Root and Stacked contexts are specified in
their own containers.
Copy link
Collaborator

@albullington albullington left a comment

Choose a reason for hiding this comment

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

I didn't get this code to run (bumping into some XCode signing issues specifically on this branch, maybe because we're using different signing provisions?), but the code looks good and I like the simplicity of this approach and having less confusing duplication of code.

@kueda
Copy link
Member Author

kueda commented Aug 30, 2024

@albullington weird, nothing should have changed with signing. Did you try running from xcode and not using the commandline react native tools? Either way, we can revisit next week after soft launch. Nothing here is critical for functionality.

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