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

Use common loader across all the pages of the application #1494

Closed
supriya3105 opened this issue Sep 29, 2023 · 19 comments
Closed

Use common loader across all the pages of the application #1494

supriya3105 opened this issue Sep 29, 2023 · 19 comments

Comments

@supriya3105
Copy link
Contributor

Is your proposal related to a problem?

Currently, the loader is added on the invoice list page and pages under settings common/loader. The same loader needs to be used across the remaining pages.

Describe the solution you'd like

Use the loader across all the pages of the application

import Loader from "common/Loader"

<div className="flex h-80v w-full flex-col justify-center">
      <Loader />
</div>

Describe alternatives you've considered

@rayy40
Copy link
Contributor

rayy40 commented Sep 29, 2023

Hey, can you assign this to me? I'd be willing to work on this.

@supriya3105
Copy link
Contributor Author

@rayyyyofsunshine Assigned the issue to you. When you raise the PR, please assign it for review to @prasanthchaduvula .

@apoorv1316
Copy link
Contributor

@rayy40 Were you able to start working on this issue? Feel free to ask for help if you are stuck somewhere.

@rayy40
Copy link
Contributor

rayy40 commented Oct 3, 2023

I am having trouble setting this up locally, I texted in discord but did not get a reply.
Can you tell me where I can ask for some help? @apoorv1316

@apoorv1316
Copy link
Contributor

@rayy40 I've replied to your query on discord.

@rayy40
Copy link
Contributor

rayy40 commented Oct 4, 2023

Hey @apoorv1316, I am trying to set it up using wsl2 by going through the docs of ubuntu, if I am unable to do so I'll update accordingly and unassign myself so that someone else can do it.

@rayy40
Copy link
Contributor

rayy40 commented Oct 5, 2023

Hey @apoorv1316, I am trying to set it up using wsl2 by going through the docs of ubuntu, if I am unable to do so I'll update accordingly and unassign myself so that someone else can do it.

Have set up the application successfully, will be raising the PR today.

@apoorv1316
Copy link
Contributor

@rayy40 Awesome. You were able to set up using WSL? If yes, would you be interested in writing the documentation for Windows using WSL

@rayy40
Copy link
Contributor

rayy40 commented Oct 5, 2023

@rayy40 Awesome. You were able to set up using WSL? If yes, would you be interested in writing the documentation for Windows using WSL

Yeah I was able to set it up using WSL. I'd be happy to tell you how I did it.

Can you help me with locating the pages? The Loader needs to be in all the pages, I am able to get the routes, but how can I get the sub routes too? Or is there a better way to do this?

@apoorv1316
Copy link
Contributor

@rayy40 Awesome. You were able to set up using WSL? If yes, would you be interested in writing the documentation for Windows using WSL

Yeah I was able to set it up using WSL. I'd be happy to tell you how I did it.

Can you help me with locating the pages? The Loader needs to be in all the pages, I am able to get the routes, but how can I get the sub routes too? Or is there a better way to do this?

@Shruti-Apte Can you respond to the above query?
@rayy40 Yes please document the steps, you can see how we have documented for Mac, Ubuntu, and Docker. Try to follow a similar approach. Thanks!
https://docs.miru.so/category/environment-setup

cc @akhilgkrishnan @prasanthchaduvula

@rayy40
Copy link
Contributor

rayy40 commented Oct 5, 2023

@apoorv1316 I will be documenting the steps by the end of the day.

And an update on the fix, I went through all the routes in the src/constants/routes file and updated the Loader for all of them.
Had some doubts related to some files, which were missing the isLoading state.

@akhilgkrishnan
Copy link
Member

@rayy40 Awesome. You were able to set up using WSL? If yes, would you be interested in writing the documentation for Windows using WSL

Yeah I was able to set it up using WSL. I'd be happy to tell you how I did it.
Can you help me with locating the pages? The Loader needs to be in all the pages, I am able to get the routes, but how can I get the sub routes too? Or is there a better way to do this?

@Shruti-Apte Can you respond to the above query? @rayy40 Yes please document the steps, you can see how we have documented for Mac, Ubuntu, and Docker. Try to follow a similar approach. Thanks! https://docs.miru.so/category/environment-setup

cc @akhilgkrishnan @prasanthchaduvula

This is a great thing. @rayy40 You can work on the documentation in a seperate PR.

@apoorv1316 Can you create a separate issue for "Windows Setup"

@prasanthchaduvula
Copy link
Contributor

@apoorv1316 I will be documenting the steps by the end of the day.

And an update on the fix, I went through all the routes in the src/constants/routes file and updated the Loader for all of them. Had some doubts related to some files, which were missing the isLoading state.

Please go through the codebase, wherever component is used, and try to verify the classnames of it to make sure whether it styles properly or not and is centre aligned.

@prasanthchaduvula
Copy link
Contributor

@rayy40 Regarding missing the isLoading state check is there any other state they are using to render the Loader

@rayy40
Copy link
Contributor

rayy40 commented Oct 5, 2023

@rayy40 Regarding missing the isLoading state check is there any other state they are using to render the Loader

They are not using the Loader at all.
So I was wondering whether I should add Loader to them too? (These files don't have the isLoading state)
But it seems like they can use a Loader since they are fetching data.
Please let me know about this.

@rayy40
Copy link
Contributor

rayy40 commented Oct 5, 2023

Please go through the codebase, wherever component is used, and try to verify the classnames of it to make sure whether it styles properly or not and is centre aligned.

I have styled the Loader properly with the components used in routes.
I just had a confusion on whether to apply the Loader to those components too which doesn't have a isLoading state, and by that I mean they don't have any state to check for loading. So, should I make one? Or should I just let them be?

@prasanthchaduvula
Copy link
Contributor

@rayy40 We need a Loader on all the react route pages.

@rayy40
Copy link
Contributor

rayy40 commented Oct 8, 2023

Hey @prasanthchaduvula , I have raised the PR including the desired changes. Can you please review it?

@apoorv1316
Copy link
Contributor

@rayy40 Thanks for working on the issue. There are more unassigned issues. Feel free to pick one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants