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

Fix: use outlet directly if route without element #8497

Merged
merged 2 commits into from Feb 25, 2022
Merged

Fix: use outlet directly if route without element #8497

merged 2 commits into from Feb 25, 2022

Conversation

liuhanqu
Copy link
Contributor

Fix #8492

@timdorr
Copy link
Member

timdorr commented Dec 17, 2021

Can you add a test to go with this?

@liuhanqu
Copy link
Contributor Author

Ok, I will add test later.

timdorr
timdorr approved these changes Dec 20, 2021
View reviewed changes
Copy link
Member

@timdorr timdorr left a comment

Choose a reason for hiding this comment

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

LGTM. Not sure if this needs MJ/RF's approval, but I went ahead and requested it anyways.

@michal-bednarz
Copy link

Hey @timdorr, any chances to get that PR merged? I have the same issue with the Outlet context & nested route without the element prop. Flattening structure helps, but I rather have my routes nested :)

JBaczuk
JBaczuk reviewed Feb 16, 2022
View reviewed changes
Copy link

@JBaczuk JBaczuk left a comment •
edited

Choose a reason for hiding this comment

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

Approved! (Seriously though, tested, works, and this is exactly what we need)

@benfiglin
Copy link

Just found this, and this is the fix I need! Nested routes are quite useless to me without this. Would be great if you could merge it as soon as possible, thanks!

mjackson
mjackson approved these changes Feb 24, 2022
View reviewed changes
Copy link
Member

@mjackson mjackson left a comment

Choose a reason for hiding this comment

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

Looks good, and thank you for the tests! We just need to fix up the PR title before merging.

@liuhanqu liuhanqu changed the title Use outlet directly if route without element Fix: use outlet directly if route without element Feb 25, 2022
@timdorr
Copy link
Member

timdorr commented Feb 25, 2022

Looks like it's good now. Thanks!

@timdorr timdorr merged commit bef3166 into remix-run:dev Feb 25, 2022
@liuhanqu liuhanqu deleted the hotfix/outlet branch March 31, 2022 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timdorr timdorr timdorr approved these changes

@mjackson mjackson mjackson approved these changes

@JBaczuk JBaczuk JBaczuk approved these changes

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

Successfully merging this pull request may close these issues.

None yet
6 participants
@liuhanqu @timdorr @michal-bednarz @benfiglin @mjackson @JBaczuk