feat: add loading animation #232
Conversation
- after resolving merge conflict
| @@ -0,0 +1,20 @@ | |||
| import React from "react"; | |||
There was a problem hiding this comment.
I think it is ok to replace the 'loading...' message with a spinner. But for the moment I'd say we could just use the CircularProgress directly, without the need for a wrapper, this gives us more control on the spacing, size, etc. Please instead of importing this component, use the CircularProgress.
| return ( | ||
| <Box | ||
| sx={{ | ||
| width: "100%", |
There was a problem hiding this comment.
is this width:100% needed? from what I could see it is a div by default
| sx={{ | ||
| width: "100%", | ||
| textAlign: "center", | ||
| py: 10, |
There was a problem hiding this comment.
please when replacing the Loader component adapt the margins to the places where used, for example, having py:10 feels like a lot of space in the children or parent list. Also, may be better to use my.
| width: "100%", | ||
| textAlign: "center", | ||
| py: 10, | ||
| m: 0, |
There was a problem hiding this comment.
is it needed to override the margins to 0?
|
Sorry for the unnecessary css! I have made the changes you suggested. Also reduced margin value in child components. |
NihalRadhakrishna
requested review from
nobeeakon
and removed request for
aadarsh-ram and
diivi
|
@NihalRadhakrishna I think the Errors page loader, has still not be changed. Please do check that out. |
| @@ -87,9 +88,14 @@ const ListEntryChildren = ({ url, urlPrefix, setUpdateNodeChildren }) => { | |||
| } | |||
| if (isPending) { | |||
There was a problem hiding this comment.
@nobeeakon I am taking this PR
I can see the 2 loading spins when we want to edit a node.
Do we want a circular progress when we are waiting for children and parents ? Or do we only need a circular progress for the whole page ?
I ask these questions to know if I delete these circular progress components in ListEntryChildren and ListEntryParents. If we want to keep them, we will display both of them or do we only want one for both lists ?
There was a problem hiding this comment.
@Piv94165 let's keep it simple ! Spinners are not the main UX blocker right now ;-)
|
I close, this was resolved here : #232 |
|
BTW thanks @NihalRadhakrishna for participating :-) |
What
Fixes bug(s)
Part of