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

Care app reloads upon login #6665

Closed
rithviknishad opened this issue Nov 17, 2023 · 7 comments · Fixed by #6828
Closed

Care app reloads upon login #6665

rithviknishad opened this issue Nov 17, 2023 · 7 comments · Fixed by #6828
Assignees
Labels
good first issue Good for newcomers

Comments

@rithviknishad
Copy link
Member

rithviknishad commented Nov 17, 2023

Describe the bug
When we login to care, the app reloads (as window.location.href=... is done instead of nvaigate(...). This causes to load the app again / fetch index.html, index.js, and other files all again.

It'd be much faster if we could avoid this from being reloaded.

To Reproduce
Steps to reproduce the behavior:

  1. Open network tab and observe the files that are being fetched.
  2. Open CARE
  3. Login to CARE using valid credentials

Expected behavior
Should not reload the entire app upon valid login.

@rithviknishad rithviknishad added the good first issue Good for newcomers label Nov 17, 2023
@AshrafMd-1
Copy link
Contributor

I want to take up this issue can u assign this to me

@ayush-seth
Copy link
Contributor

Any reason this project is using raviger which barely anyone has ever heard of and has not been updated since 2022 instead of using the industry standard React Router?

@ayush-seth
Copy link
Contributor

This is a bigger problem at its core with how route protection and login is being handled and just replacing window.location.href = with navigate() is not going to solve these issues.

For ex. when I am logged out and I navigate to /shifting/board, instead of being redirected to /login I stay on /shifting/board and in those cases, after logging in, it does window.location.href = window.location.pathname.toString(), using navigate here won't do anything because the path does not change, you have to reload the whole page. Same goes for any other url.

@rithviknishad
Copy link
Member Author

I don't see any issue with Raviger's routing.

If you look at the code, we have two routers: Session and App router. App router is accessible only if the query to get the current profile responds with 200, you'll be exposed to the App router, else it'll show the Session router.

We just need to invoke the refetch of the current user query somehow whenever the JWT access token is updated in the local storage. That'd cause the AuthUserProvider component to render, and hence exposing the appropriate router based on the response.

image image

@Ashesh3
Copy link
Member

Ashesh3 commented Nov 29, 2023

@AshrafMd-1 What are the updates on this issue?

@AshrafMd-1
Copy link
Contributor

@Ashesh3 , I am currently addressing a p2 issue. Once I have resolved that, I will proceed to work on and submit a pull request soon.

@AshrafMd-1
Copy link
Contributor

@rithviknishad can you unassign me this issue as I am unable to find an optimal solution for the problem

The process is to refetch the current user query when the user logins correctly

My attempts to this were:

  • Passed the refetch option as props, but it resulted in the code becoming quite messy.
  • Used the current URL as a condition for refetch but doing so caused many requests each time the url changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants