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

Loading indicator till app inits #3788

Merged
merged 3 commits into from
May 13, 2019
Merged

Loading indicator till app inits #3788

merged 3 commits into from
May 13, 2019

Conversation

ranbena
Copy link
Contributor

@ranbena ranbena commented May 12, 2019

  • Feature

Description

Before app loads there might be a substantial wait for <app-view> to populate. Instead of looking at a blank screen, here's a nice hovering logo.

This is a css+html only change 😛
Even determining indicator hide/show is done with css, according to <app-view> being empty/full.

Some notes

I created a new beautiful sleek animation with the bars but for some reason it's animation got stuck on the main thread. Don't know how since I used only gpu accelerated transition props. Anywho, this is good enough.

I only wish I didn't have to stick the entire html elements into index.html and it's twin brother.
Lmk if there's a way around that (independent of Angular/React).

Mobile & Desktop Screenshots/Recordings

Before:

After:

@ranbena ranbena self-assigned this May 12, 2019
@ranbena ranbena marked this pull request as ready for review May 12, 2019 14:18
app-view:not(:empty) ~ .loading-indicator {
opacity: 0;
transform: scale(0.9);
pointer-events: none;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unnecessary but it helps me sleep at night.
Better make sure transparent elements are absolutely non-interactable.

@arikfr
Copy link
Member

arikfr commented May 12, 2019

Is the E2E fail due to the CSS changes?

@ranbena
Copy link
Contributor Author

ranbena commented May 12, 2019

Is the E2E fail due to the CSS changes?

Nope. I'm looking into it.

client/app/assets/less/redash/css-logo.less Outdated Show resolved Hide resolved
client/app/assets/less/redash/css-logo.less Outdated Show resolved Hide resolved
client/app/index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

@kravets-levko kravets-levko left a comment

Choose a reason for hiding this comment

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

Looks nice 👍

@ranbena ranbena merged commit 95f11e6 into master May 13, 2019
@ranbena ranbena deleted the loading-indicator branch May 13, 2019 18:11
harveyrendell pushed a commit to pushpay/redash that referenced this pull request Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants