mrpmorris / Fluxor

Fluxor is a zero boilerplate Flux/Redux library for Microsoft .NET and Blazor.
MIT License
1.22k stars 139 forks source link

Please review PR 454 (Uri.Compare, routes with #fragments) #455

Closed dmitry-pavlov closed 3 months ago

dmitry-pavlov commented 8 months ago

@mrpmorris please review #454 once you have time and consider for merging. TIA!

mrpmorris commented 8 months ago

Sorry for the delayed response, life got in the way!

Could put the details of the problem into the ticket, please? I don't know what is wrong with the current code.

Could you please also add one or more unit tests?

dmitry-pavlov commented 8 months ago

I added that to handle routes Uri links as Uri links rather than as strings. Reason was the links with #fragments if I remember right.

As for the unit tests, sure, I will have a look what I can add. Perhaps, this weekend. Thanks!

mrpmorris commented 8 months ago

Thank you, I appreciate your contribution and your patience!

dmitry-pavlov commented 6 months ago

Hey @mrpmorris Sorry for delay. I got stuck figuring out what kind of test it could be... Any ideas are welcome.

Let me put some light on the reason why I needed that. Maybe it would be enough to merge this PR.

Blazor can't handle routes with fragments out of the box https://chrissainty.com/fragment-routing-with-blazor/

Saying fragments, I mean those URI Fragment part of anchor links to jump to a specific part of a page like this below:

<a href="#anchor-name">Jump to the part of the page with the “anchor-name” id </a>

In the app I am working on I have a page with tabs, which are handled by Blazor and use Fluxor.Blazor.Web.Middlewares.Routing.GoAction and routing middleware to navigate between tabs, and identify the which NavLink is active at the moment (to highlight on UI). In addition to that I have on-page navigation with "fragments" pointing to some element IDs on the page. I am trying to reduce using NavigationManager directly and instead - use routing middleware to handle in app navigation. So all navigation (without force page reloading and with it (when needed) is built on top of Fluxor.Blazor.Web.Middlewares.Routing classes.

To check if the route is the same in HandleGoActionAsync you just strings from NavigationManager.Uri and let it be navigated if the strings are different. For links with URI fragment it could a bit be more sophisticated if UrlComparer would sompare URIs rather than strings.

Please let me know what sort of test I could add for this PR, or feel free to merge the PR. Thanks!

mrpmorris commented 3 months ago

I don't know what unit tests you can add. I usually look at the unit tests to understand the purpose of the code.

I'm putting together V6 at the moment, don't miss out :)