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

Add LoadingTemplate delayed/failed updates #3233

Merged
merged 1 commit into from
Mar 14, 2022

Conversation

dit7ya
Copy link
Contributor

@dit7ya dit7ya commented Mar 9, 2022

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.

2022-03-09_22-03
2022-03-09_22-03_1

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.

@dit7ya dit7ya requested a review from a team March 9, 2022 16:37
@dit7ya dit7ya self-assigned this Mar 9, 2022
@dit7ya dit7ya added the feature label Mar 9, 2022
@dit7ya dit7ya changed the title Feat: add LoadingTemplate delayed/failed updates Add LoadingTemplate delayed/failed updates Mar 10, 2022
Copy link
Member

@arrenv arrenv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While hard to test accurately, unless you have some ideas. It looks to be working nice, good stuff.

loader-error-state

Copy link
Contributor

@ArmandoGraterol ArmandoGraterol left a 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

Copy link
Contributor

@chinins chinins left a 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.

@@ -0,0 +1,19 @@
<svg width="223" height="116" viewBox="0 0 223 116" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Contributor

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.

@dit7ya
Copy link
Contributor Author

dit7ya commented Mar 11, 2022

@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 naked-mole.svg in the same folder has 0 0 600 150 as viewbox and neither do I have much idea about the implications of this. What do you suggest as the values? I am going to prettify that file btw.

Copy link
Contributor

@chinins chinins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@@ -0,0 +1,19 @@
<svg width="223" height="116" viewBox="0 0 223 116" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Contributor

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.

@dit7ya dit7ya force-pushed the feat/3229-loading-delay-failing branch from 6df337a to 4b113a2 Compare March 14, 2022 05:22
@dit7ya dit7ya force-pushed the feat/3229-loading-delay-failing branch from 4b113a2 to 4f1582f Compare March 14, 2022 05:25
@dit7ya dit7ya merged commit 3d13f37 into master Mar 14, 2022
@dit7ya dit7ya deleted the feat/3229-loading-delay-failing branch March 14, 2022 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add dapp loading delaying and failed updates
4 participants