-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add LoadingTemplate delayed/failed updates #3233
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 It was indeed difficult to test, I had to manually change the conditional to always show the loading state in order to actually see the different components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's looking good, just have a question about svgs.
src/img/naked-mole-without-bg.svg
Outdated
@@ -0,0 +1,19 @@ | |||
<svg width="223" height="116" viewBox="0 0 223 116" fill="none" xmlns="http://www.w3.org/2000/svg"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am no expert about svgs but I know that we always try to keep viewbox to 0 0 30 30
(please, see other svg examples in the repo). Is there any reason we can't have viewbox with these values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the 0 0 30 30
here is not needed as this isn't an icon, so therefore it doesn't follow the 30x30 size standard that we have for icons in the dApp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we have it for all svgs, but the other naked mole doesn't have it indeed. Thank you for clarification.
@arrenv and @ArmandoGraterol, there is a Network Throttle setting in the browser dev tools which can be used to test these kind of things (You need to enable the throttle after letting the page load once and refresh though, otherwise the initial SPA will take forever to load). However, while developing I followed what Armando mentioned for quick iterations. https://developer.chrome.com/docs/devtools/network/#throttle @chinins I just copied the svg from Figma. The other |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
src/img/naked-mole-without-bg.svg
Outdated
@@ -0,0 +1,19 @@ | |||
<svg width="223" height="116" viewBox="0 0 223 116" fill="none" xmlns="http://www.w3.org/2000/svg"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we have it for all svgs, but the other naked mole doesn't have it indeed. Thank you for clarification.
6df337a
to
4b113a2
Compare
4b113a2
to
4f1582f
Compare
Description
Adds status updates in the LoadingTemplate components - as per the specification from the issue. The timers are of 15 seconds and 30 seconds for delaying and failing respectively.
Note: The timer starts once the main script (or this component)) is loaded.
Since there are just two issues in this project I have just left
master
as the base branch. If you think there should be a feature branch, please let me know.Resolves #3229.