-
Notifications
You must be signed in to change notification settings - Fork 199
fix(flow-client): loading state muting based on trigger events #24230
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
d78c945
78f3661
980cb06
376b93e
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,134 @@ | ||
| /* | ||
| * Copyright 2000-2026 Vaadin Ltd. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); you may not | ||
| * use this file except in compliance with the License. You may obtain a copy of | ||
| * the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
| * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
| * License for the specific language governing permissions and limitations under | ||
| * the License. | ||
| */ | ||
| package com.vaadin.client.communication; | ||
|
|
||
| import com.google.gwt.core.client.Scheduler; | ||
|
|
||
| import com.vaadin.client.ConnectionIndicator; | ||
| import com.vaadin.client.Registry; | ||
| import com.vaadin.client.flow.collection.JsCollections; | ||
| import com.vaadin.client.flow.collection.JsSet; | ||
| import com.vaadin.flow.shared.JsonConstants; | ||
|
|
||
| /** | ||
| * Manages the state of loading indicator based on active RPC requests, event | ||
| * types, and lifecycle events. | ||
| * <p> | ||
| * This class ensures appropriate visual feedback (e.g., loading bar) is shown | ||
| * or hidden according to the current network conditions and request status. It | ||
| * is responsible for muting the loading indication when RPC requests are | ||
| * triggered by high-frequency UI events (mousemove and such) to avoid excessive | ||
| * visual noise in these cases. | ||
| */ | ||
| public class LoadingIndicatorStateHandler { | ||
| private final Registry registry; | ||
|
|
||
| private boolean loading = false; | ||
|
|
||
| private boolean showLoading = false; | ||
|
|
||
| // High-frequency events, whose related RPC requests are not expected | ||
| // to trigger loading indication. | ||
| private static final JsSet<String> SILENT_EVENT_TYPES = JsCollections.set(); | ||
| { | ||
| JsCollections.array("keydown", "keypress", "keyup", "mousemove", | ||
| "pointermove", "pointerrawupdate", "touchmove", "beforeinput", | ||
| "input", "scroll", "wheel", "drag", "dragover") | ||
| .forEach(SILENT_EVENT_TYPES::add); | ||
| } | ||
|
|
||
| /** | ||
| * Creates a new instance connected to the given registry. | ||
| * | ||
| * @param registry | ||
| * the global registry | ||
| */ | ||
| public LoadingIndicatorStateHandler(Registry registry) { | ||
| this.registry = registry; | ||
| } | ||
|
|
||
| /** | ||
| * Updates the connection state to {@link ConnectionIndicator#LOADING} when | ||
| * a non-silent request starts. | ||
| */ | ||
| public void startLoading() { | ||
| if (!showLoading) { | ||
| // The next request is muted, do not show loading. | ||
| return; | ||
| } | ||
|
|
||
| update(); | ||
| } | ||
|
|
||
| /** | ||
| * Updates the connection state to {@link ConnectionIndicator#CONNECTED} | ||
| * when active requests finish. | ||
| */ | ||
| public void stopLoading() { | ||
| if (registry.getRequestResponseTracker().hasActiveRequest()) { | ||
| // Some request is in progress, skip the current stop. | ||
| return; | ||
| } | ||
|
|
||
| // Reset the loading state | ||
| showLoading = false; | ||
|
|
||
| // Debounce the update to avoid hiding loading when a follow-up | ||
| // request is started or scheduled right away. | ||
| Scheduler.get().scheduleDeferred(this::update); | ||
| } | ||
|
|
||
| /** | ||
| * Processes an RPC message to determine if a loading indicator should be | ||
| * displayed. | ||
| * | ||
| * @param rpcType | ||
| * the type of RPC request being processed | ||
| * @param eventType | ||
| * for event RPC requests, the name of the event, otherwise | ||
| * {@code null} | ||
| */ | ||
| public void processMessage(String rpcType, String eventType) { | ||
| // Require at least one non-silent message to indicate loading for | ||
| // the next request. | ||
| boolean silent = JsonConstants.RPC_EVENT_TYPE.equals(rpcType) | ||
| && eventType != null && SILENT_EVENT_TYPES.has(eventType); | ||
| if (!silent) { | ||
| showLoading = true; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Applies the loading state change after a dirty check. | ||
| */ | ||
| private void update() { | ||
| if (showLoading == loading) { | ||
| return; | ||
| } | ||
|
|
||
| loading = showLoading; | ||
| // Setting the loading state directly using | ||
| // `ConnectionIndicator.setState()` interferes with other loading | ||
| // parties | ||
| // (Flow router, Hilla requests), therefore `.loadingStarted()` / | ||
| // `.loadingFinished()` are preferred. | ||
| if (loading) { | ||
| ConnectionIndicator.loadingStarted(); | ||
| } else { | ||
| ConnectionIndicator.loadingFinished(); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,6 @@ | |
| import com.google.gwt.core.client.GWT; | ||
| import com.google.gwt.user.client.Timer; | ||
|
|
||
| import com.vaadin.client.ConnectionIndicator; | ||
| import com.vaadin.client.Console; | ||
| import com.vaadin.client.Registry; | ||
| import com.vaadin.flow.shared.ApplicationConstants; | ||
|
|
@@ -137,7 +136,6 @@ private void doSendInvocationsToServer() { | |
| + pushPendingMessage.toJson()); | ||
| JsonObject payload = pushPendingMessage; | ||
| pushPendingMessage = null; | ||
| registry.getRequestResponseTracker().startRequest(); | ||
|
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. Can you explain why the call to
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. Having multiple calls to The In this instance, the In the Now that the code path is cleaned up a bit, it would rather make sense to me to remove if guarding against the “another request active” exception around
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. Probably it can be removed, but I have some concerns about the "reconnect" path. |
||
| sendPayload(payload); | ||
| return; | ||
| } else if (hasQueuedMessages()) { | ||
|
|
@@ -156,7 +154,6 @@ private void doSendInvocationsToServer() { | |
| return; | ||
| } | ||
|
|
||
| boolean showLoadingIndicator = serverRpcQueue.showLoadingIndicator(); | ||
| JsonArray reqJson = serverRpcQueue.toJson(); | ||
| serverRpcQueue.clear(); | ||
|
|
||
|
|
@@ -177,9 +174,7 @@ private void doSendInvocationsToServer() { | |
| resetTimer(); | ||
| extraJson.put(ApplicationConstants.RESYNCHRONIZE_ID, true); | ||
| } | ||
| if (showLoadingIndicator) { | ||
| ConnectionIndicator.setState(ConnectionIndicator.LOADING); | ||
| } | ||
| registry.getLoadingIndicatorStateHandler().startLoading(); | ||
| send(reqJson, extraJson); | ||
| } | ||
|
|
||
|
|
@@ -193,7 +188,6 @@ private void doSendInvocationsToServer() { | |
| */ | ||
| protected void send(final JsonArray reqInvocations, | ||
| final JsonObject extraJson) { | ||
| registry.getRequestResponseTracker().startRequest(); | ||
| send(preparePayload(reqInvocations, extraJson)); | ||
| } | ||
|
|
||
|
|
||
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.
Shouldn't it be
RPC_TYPE_EVENT? The constants have the same "event" value, butRPC_EVENT_TYPEseems incorrect here.