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

A partial implementation of the router and associated bits #107

Merged
merged 10 commits into from
Jan 18, 2022
Merged

A partial implementation of the router and associated bits #107

merged 10 commits into from
Jan 18, 2022

Conversation

autarch
Copy link
Contributor

@autarch autarch commented Jan 10, 2022

No description provided.

@autarch autarch changed the title Add title prop to Link A partial implementation of the router and associated bits Jan 10, 2022
@autarch autarch force-pushed the autarch/half-assed-router branch from e6722c5 to 7732ddc Compare January 10, 2022 06:00
@jkelleyrtp
Copy link
Member

jkelleyrtp commented Jan 10, 2022 •
edited
Loading

Thank you!!
When you get a chance, you can rebase to get the fixes from #108

@autarch autarch force-pushed the autarch/half-assed-router branch 2 times, most recently from 9ded513 to fa9d082 Compare January 10, 2022 15:42
@autarch
Copy link
Contributor Author

autarch commented Jan 10, 2022

Thank you!! When you get a chance, you can rebase to get the fixes from #108

Done.

@autarch autarch force-pushed the autarch/half-assed-router branch from fa9d082 to f526c7d Compare January 10, 2022 22:26
@autarch
Add title prop to Link
e22ba5b 
Every element can have a title, but it's particularly useful on links, so I
think making it an explicit option is worthwhile.
@autarch
Fix typo in RouterService struct's "registered_routes" field name
d367e0f 
This was "registerd_routes", missing an "e".
@autarch
Add more trace messages to the RouterService code
3a5b417 
I was trying to debug some issues with my routes and this additional tracing
was quite helpful.
@autarch
Commit WIP on router
3c6142f 
There are a few different changes in here that probably need to be picked
apart. I'm sure much of this is wrong.

* Fix missing `dyn` that compiler complained about in router.rs

* Make UseRoute store a `Rc<RouterService>` rather than a string so we can get
information out of the router like current location.

* Implement `UseRoute`'s nth_segment and last_segment methods. I changed the
return type to a String because of the above.

* Remove some unused imports in platform/mod.rs and service.rs

* Implement the `use_route` fn. It panics if called outside a Router { } (I
think). I think that makes sense.

* Add a `current_location` method to `RouterService` that returns the current
location. I needed this both for the `UseRoute` implementation and _also_ so I
could get at this in my webapp code. I think having some way to get this will
be useful for others, whether or not this exact API is used. In my case, I
want to compare the current path to the `to` path of a `Link` so I can use a
different class for that `Link` if it is the currently active page.
@autarch
Implement router matching for path parameters
f8a7e1c 
We don't want to have the router just always match paths as exact strings. If
a path contains a parameter like "/thing/:id" then the ":id" portion of the
route should match _any_ string, not a literal ":id".
@autarch
Implement UseRoute segment method
c9408da 
This relies on the RouterService to capture path params when it does path
matching.
@autarch autarch force-pushed the autarch/half-assed-router branch from 1a50df7 to 3397a2c Compare January 13, 2022 18:21
@autarch autarch force-pushed the autarch/half-assed-router branch from 3397a2c to e06eac1 Compare January 13, 2022 18:26
@autarch
Fix handling of re-renders in the Router
81c094e 
Previously the router just stored a `root_found` boolean after it picked a
route. But on re-render it would just always return false from `should_render`
if this was true. This just aborted routing after a future resolved (or
anything else that triggered a re-render).

Now we store the matching ScopeId and check that against our routes in a
re-render so we actually do re-render the matching route.
@autarch
Don't expect all components to have a scope in ScopeArena.ensure_drop…
9b282d8 
…_safety

Not all components will be mounted when using a Router, so we cannot assume
all components have a scope.
@jkelleyrtp
Copy link
Member

Would you like me to review this? Looks pretty functional :)

@autarch
Copy link
Contributor Author

autarch commented Jan 17, 2022

Would you like me to review this? Looks pretty functional :)

Yes, please do. I think there's enough here to at least figure out if this is the right design. I can make further changes or I'm happy to have you make commits directly. I can also rebase this as needed.

@autarch autarch marked this pull request as ready for review January 17, 2022 21:51
@autarch autarch force-pushed the autarch/half-assed-router branch from d562241 to 6408058 Compare January 17, 2022 21:52
@jkelleyrtp
Copy link
Member

Since the Router still needs some configuration for desktop support, I'm going to merge this as is and then follow it up with a PR that you can review.

Thank you for adding this! It's amazing to have full (almost) router support now!

@jkelleyrtp jkelleyrtp merged commit 8d3ac3f into DioxusLabs:master Jan 18, 2022
@autarch autarch deleted the autarch/half-assed-router branch January 28, 2022 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@autarch @jkelleyrtp