Skip to content

isActive for dynamic index routes #2425

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
Merged
merged 3 commits into from
Oct 30, 2015
Merged

isActive for dynamic index routes #2425

merged 3 commits into from
Oct 30, 2015

Conversation

taion
Copy link
Contributor

@taion taion commented Oct 30, 2015

Fixes #2072

@taion taion mentioned this pull request Oct 30, 2015
Closed
@taion taion added this to the v1.0.0-final milestone Oct 30, 2015
@taion
Copy link
Contributor Author

taion commented Oct 30, 2015

Can we make the coveralls check be a little more lenient?

A simpler implementation might be possible - make getMatchingRoute be e.g. getMatchingRouteIndex and just verify that all remaining routes have no path specified. For completeness we would then maintain the invariant in getIndexRoute in matchRoutes (or elsewhere) that index routes do not have path specified.

I don't want the merge conflict with #2421 though.

@taion
Copy link
Contributor Author

taion commented Oct 30, 2015

Updated with a much simpler check that I believe covers all the relevant cases.

@knowbody
Copy link
Contributor

thanks @taion! I like it!

also, yeah we should change the coveralls settings

knowbody added a commit that referenced this pull request Oct 30, 2015
@knowbody
Merge pull request #2425 from taion/dynamic-indexRoute
26c632a 
isActive for dynamic index routes
@knowbody knowbody merged commit 26c632a into remix-run:master Oct 30, 2015
@taion
Copy link
Contributor Author

taion commented Oct 30, 2015

Ack I wanted to squash this first. Oh well.

@taion taion deleted the dynamic-indexRoute branch October 30, 2015 21:09
This was referenced Oct 31, 2015
Merged
Merged
@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.0.0-final
Development

Successfully merging this pull request may close these issues.
2 participants
@taion @knowbody