Repository navigation
#435: show spinner indicating page is loading #449
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
base: main
Are you sure you want to change the base?
Changes from all commits
b014b29
8d88bd1
2f86194
2679ec4
b63c35b
5a7379d
b909159
63627f5
c054445
ec4707c
f6e4729
416240b
ed03b1e
0a680b4
0bd66fb
5045a4f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # SQLPage display progress | ||
|
|
||
| This is a very simple example of a page that shows a spinner to indicate page is loading. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| -- Disable actix-web compression middleware: | ||
| -- https://github.com/actix/actix-web/issues/3410 | ||
| SELECT 'http_header' AS component, 'Identity' AS "Content-Encoding"; | ||
|
|
||
| SELECT 'shell' AS component, 'light' AS theme; | ||
|
|
||
| -- can disable the spinner to show only progress bar | ||
| SELECT 'loader-start' AS component, '' AS spinner; | ||
|
|
||
| SELECT 'progress' AS component, | ||
| NULL AS percent, | ||
| 'sm' AS size, | ||
| 'yellow' AS color, | ||
| 'Working on it' AS stage; | ||
| SELECT sqlpage.fetch('https://example.com'); | ||
|
|
||
| SELECT 'loader-stop' AS component; | ||
|
|
||
| SELECT 'text' AS component, 'Processing complete.' AS contents; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| -- Disable actix-web compression middleware: | ||
| -- https://github.com/actix/actix-web/issues/3410 | ||
| SELECT 'http_header' AS component, 'Identity' AS "Content-Encoding"; | ||
|
|
||
| SELECT 'shell' AS component, 'dark' AS theme; | ||
|
|
||
| SELECT 'loader-start' AS component, | ||
| -- pick from the tabler spinners: https://tabler.io/docs/components/spinners | ||
| "spinner-border" spinner, | ||
| "sm" AS size, | ||
| "red" AS color; | ||
|
|
||
| SELECT 'progress' AS component, | ||
| 'sm' AS size, | ||
| 'yellow' AS color, | ||
| '0' AS percent, | ||
| 'Fething data' AS stage; | ||
| SELECT sqlpage.fetch('https://example.com'); | ||
|
|
||
| /* percent property is optional */ | ||
| SELECT 'progress' AS component, | ||
| NULL AS percent, | ||
| 'sm' AS size, | ||
| 'yellow' AS color, | ||
| 'Doing something' AS stage; | ||
| SELECT sqlpage.fetch('https://example.com'); | ||
|
|
||
| /* stage property is optional */ | ||
| SELECT 'progress' AS component, | ||
| 'sm' AS size, | ||
| 'yellow' AS color, | ||
| '40' AS percent; | ||
| SELECT sqlpage.fetch('https://example.com'); | ||
|
|
||
| /* multiple rows */ | ||
| SELECT 'progress' AS component, 'sm' AS size, 'yellow' AS color; | ||
| SELECT '70' AS percent, 'Fetching data' AS stage | ||
| SELECT sqlpage.fetch('https://example.com'); | ||
| SELECT '80' AS percent, 'Fetching more data' AS stage; | ||
| SELECT sqlpage.fetch('https://example.com'); | ||
| SELECT '90' AS percent, 'Fetching again' AS stage; | ||
| SELECT sqlpage.fetch('https://example.com'); | ||
|
|
||
| SELECT '100' AS percent; | ||
|
|
||
| SELECT 'loader-stop' AS component; | ||
|
|
||
| SELECT 'text' AS component, | ||
| 'It works!' AS title, | ||
| TRUE AS center, | ||
| 'Page is loaded.' AS contents; | ||
|
|
||
| SELECT 'button' AS component; | ||
| SELECT 'Go' AS title, './go.sql' AS link; | ||
|
|
||
| -- can use progress on it's own | ||
| SELECT 'progress' AS component, 'sm' AS size, 'Waiting for user' AS stage; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| <div {{#if id}}id="{{id}}"{{/if}} class="sqlpage-loader-start"> | ||
| <div class="sqlpage-loader-container"> | ||
| <span class="{{default spinner "spinner-border"}} | ||
| {{#if size}}spinner-border-{{size}}{{/if}} | ||
| {{#if color}}text-{{color}}{{/if}}"> | ||
| </span> | ||
|
Comment on lines
+1
to
+6
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. errors between
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I knew about this one. I didn't even begin to think about how to solve that... lol, just make sure it doesn't error.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can fix it by closing the <div {{#if id}}id="{{id}}"{{/if}} class="sqlpage-loader" role="status">
<span class="{{default spinner "spinner-border"}} {{#if color}}text-{{color}}{{/if}}"></span>
</div>
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could be forgetting, but I thought the whole hack here was leaving the div open so the spinner disappears when it's closed... My CSS evolved over time though, and it looks as though closing the div should work. I just recall leaving it open was important for some reason. It's closed in the sqlpage-loader-stop element. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| </div> | ||
| </div> | ||
| <div class="sqlpage-loader-stop"></div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| <div class="sqlpage-progress-container"> | ||
| <div class="progress {{~#if size}} progress-{{size}}{{/if}}"> | ||
| <div | ||
| id="sqlpage-loading-{{default stage "progress"}}" | ||
| class="progress-bar | ||
| {{~#if (not percent)}} progress-bar-indeterminate{{/if}} | ||
| {{~#if color}} bg-{{color}}{{/if}}" | ||
| role="progressbar" | ||
| aria-valuenow="{{percent}}" | ||
| {{~#if percent}}style="width: {{percent}}%; display: block"{{/if}} | ||
|
Comment on lines
+6
to
+10
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| aria-valuemin="0" aria-valuemax="100" | ||
| {{~#if stage}}aria-label="{{stage}}"{{/if}}> | ||
| </div> | ||
| </div> | ||
| {{~#if stage}} | ||
| <br> | ||
| <label for="sqlpage-loading-{{default stage "progress"}}">{{stage}}</label> | ||
| {{/if}} | ||
|
Comment on lines
+15
to
+18
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. labels don't work on divs iirc https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/label |
||
| </div> | ||
| {{#each_row}} | ||
| {{#if (or percent stage)}} | ||
| <div class="sqlpage-progress-container"> | ||
| <div class="progress {{~#if ../size}} progress-{{../size}}{{/if}}"> | ||
| <div | ||
| id="sqlpage-loading-{{default stage "progress"}}" | ||
| class="progress-bar | ||
| {{~#if (not percent)}} progress-bar-indeterminate{{/if}} | ||
| {{~#if ../color}} bg-{{../color}}{{/if}}" | ||
| role="progressbar" | ||
| aria-valuenow="{{percent}}" | ||
| {{~#if percent}}style="width: {{percent}}%; display: block"{{/if}} | ||
| aria-valuemin="0" aria-valuemax="100" | ||
| {{~#if stage}}aria-label="{{stage}}"{{/if}}> | ||
| </div> | ||
| </div> | ||
| {{~#if stage}} | ||
| <br> | ||
| <label for="sqlpage-loading-{{default stage "progress"}}">{{stage}}</label> | ||
| {{/if}} | ||
| </div> | ||
| {{/if}} | ||
| {{/each_row}} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| SELECT 'shell' AS component, 'Progress test' AS title; | ||
|
|
||
| SELECT 'loader-start' AS component, 'completed-loader' AS id; | ||
| SELECT 'loader-stop' AS component; | ||
|
|
||
| SELECT 'progress' AS component, 25 AS percent, 'blue' AS color; | ||
| SELECT 100 AS percent; | ||
|
|
||
| SELECT 'loader-start' AS component, 'active-loader' AS id, 'red' AS color; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import { expect, test } from "../../fixture"; | ||
|
Check failure on line 1 in tests/end-to-end/fixtures/progress/test.ts
|
||
|
|
||
| test("stops completed loaders and shows only the latest progress", async ({ | ||
| page, | ||
| }) => { | ||
| await expect(page.locator("#completed-loader")).toBeHidden(); | ||
| await expect(page.locator("#active-loader .spinner-border")).toBeVisible(); | ||
| await expect( | ||
| page.locator(".sqlpage-progress-container").first(), | ||
| ).toBeHidden(); | ||
| const progress = page.getByRole("progressbar"); | ||
| await expect(progress).toHaveCount(1); | ||
| await expect(progress).toHaveAttribute("aria-valuenow", "100"); | ||
| await expect(progress).toHaveClass(/bg-blue/); | ||
| }); | ||
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.
do we need this still? actix/actix-web#341 is closed
Uh oh!
There was an error while loading. Please reload this page.
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 not immediately obvious to me how actix/actix-web#341 is related.
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.
ope, sorry missed a 0 at the end
actix/actix-web#3410
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 implemented this not as a work around for some buffering issue, but because I have some legitimately long running queries so the spinner helps indicate something is happening.
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.
sure, but its fixed upstream now. I think the better approach would be to update the actix-web dependency and use the upstream fix :)
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 misunderstood your comment at the first. My bad.
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.
nw, thanks for the PR, excited to see the next revision :)
please re-request review when ready