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

(enhancement) Utilize client-side routing provided by react-router-dom in the SideNav links #974

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

coderatomy
Copy link
Collaborator

Summary

When you use the Link component from react-router-dom to create navigation links, it ensures that the navigation happens without a full page refresh. Instead of requesting a new HTML page from the server, React updates the DOM efficiently by re-rendering only the components necessary for the new route. This approach provides a smoother and faster user experience because it doesn't reload the entire page, making web applications feel more like native applications.

Changes

  • Uses react-router-dom's Link component instead of material UI's.

Screencasts

Before

Screencast.from.2023-10-26.23-02-49.webm

After

Screencast.from.2023-10-26.23-03-24.webm

@coderatomy
Copy link
Collaborator Author

coderatomy commented Oct 26, 2023

Keep the focus in the URL editor as the routes change. You will definitely notice the improvement and how slightly fast routing is handled.
Notice how we no longer have that full page reload. Check the summary above for more details

@NdibeRaymond
Copy link
Collaborator

do you have an issue created for this @coderatomy ? can you link the issue here?

@NdibeRaymond
Copy link
Collaborator

also this is a nice thing to have, but we need to make sure that there is no flow that relies and the frontend periodically fetching things like new projects, etc from the backend on reload. We might risk having stale data if that is the case. You might want to audit this to make sure that we don't risk having stale data if this is merged

@coderatomy
Copy link
Collaborator Author

coderatomy commented Nov 12, 2023

also this is a nice thing to have, but we need to make sure that there is no flow that relies and the frontend periodically fetching things like new projects, etc from the backend on reload. We might risk having stale data if that is the case. You might want to audit this to make sure that we don't risk having stale data if this is merged

I actually missed that. How about we incorporate swr for fetching and re-validating data @NdibeRaymond? But I hope this will be added in the on-going migration.

@NdibeRaymond
Copy link
Collaborator

also this is a nice thing to have, but we need to make sure that there is no flow that relies and the frontend periodically fetching things like new projects, etc from the backend on reload. We might risk having stale data if that is the case. You might want to audit this to make sure that we don't risk having stale data if this is merged

I actually missed that. How about we incorporate swr for fetching and re-validating data @NdibeRaymond? But I hope this will be added in the on-going migration.

we can start from there yeaa, but then we also need to take our time to figure out the places where we are in most danger of having stale data

@tuxology
Copy link
Member

I'm marking this as low priority as of now since this needs larger investigation and it is just a nice to have

@tuxology tuxology added low priority enhancement New feature or request labels Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request low priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants