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

Don't show loading indicators when refreshing the system status #30712

Merged
merged 14 commits into from Apr 27, 2024
2 changes: 1 addition & 1 deletion templates/admin/dashboard.tmpl
Expand Up @@ -76,7 +76,7 @@
{{ctx.Locale.Tr "admin.dashboard.system_status"}}
</h4>
{{/* TODO: make these stats work in multi-server deployments, likely needs per-server stats in DB */}}
<div hx-get="{{$.Link}}/system_status" hx-swap="morph:innerHTML" hx-trigger="every 5s" hx-indicator=".divider" class="ui attached table segment">
<div hx-get="{{$.Link}}/system_status" hx-swap="morph:innerHTML" hx-trigger="every 5s" hx-indicator=".tw-hidden" class="ui attached table segment">
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved

This comment was marked as outdated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually it could also use something like hx-indicator="no-indicator" , because no-indicator also matches nothing.

Or even as simple as hx-indicator="no", we would never use <no> tag in HTML code, so it also matches nothing.

Copy link
Member Author

Choose a reason for hiding this comment

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

But that prints an error in the console on every request

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, then tw-hidden also doesn't seem good enough because tw-hidden might also not exist.

Ideally it could be like this:

Suggested change
<div hx-get="{{$.Link}}/system_status" hx-swap="morph:innerHTML" hx-trigger="every 5s" hx-indicator=".tw-hidden" class="ui attached table segment">
<div hx-get="{{$.Link}}/system_status" hx-swap="morph:innerHTML" hx-trigger="every 5s" hx-indicator=".hidden-loading-indicator" class="ui attached table segment">

Then put <div class="hidden-loading-indicator tw-hidden"></div> into "admin/system_status"

Then everything is clear and I think it reads better.

Copy link
Contributor

Choose a reason for hiding this comment

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

And more details, .tw-hidden can't be right. It just adds the indicator to all the tw-hidden elements on this page, which causes various side effects

Think about a case:

<div class="panel tw-hidden"></div>
<div hx-get hx-indicator=".tw-hidden"></div>

The "panel" is not related to the hx-get.

When the loading-indicator is added to the "panel", and the request is still running, then the user shows the panel: surprise: they sees a "loading panel".

{{template "admin/system_status" .}}
</div>
</div>
Expand Down