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

Auto Refresh button #536

Merged
merged 2 commits into from
Nov 22, 2022

Conversation

victorgawk
Copy link
Contributor

Add an "Auto Refresh" button next to the "Force Refresh" button:

image

When pressed, the page will be automatically force refreshed every X seconds where X can be configured in the "User Preferences" panel:

image

When the auto refresh is active, the "Auto Refresh" and "Force Refresh" button will be displayed with pulsating and rotation animations respectively:

Animação

Pressing the button again disables the auto refresh.

@weeco weeco requested a review from rikimaru0345 November 13, 2022 23:33
@weeco
Copy link
Contributor

weeco commented Nov 13, 2022

Hey @victorgawk ,
the auto refresh button is a great idea - thank you for coming up with this PR!

I have two thoughts and one question:

  1. Some pages, such as the consumer groups page, are expected to load slow if you have a large number of consumer groups. That's because we have to issue a series of Kafka requests in order to collect all the showed information. Thus, we should never abort ongoing requests, and only start the interval timer after the previous request has been finished. We should also make this behaviour clear to the user, so that they are not surprised that they don't see up-to-date values in case the requests take a long time.
  2. We should enforce a minimum auto refresh interval of 5seconds

Question: On the topics page with messages, would this also retrigger the search over and over again? This may be undesired behaviour by a user.

@victorgawk
Copy link
Contributor Author

Question: On the topics page with messages, would this also retrigger the search over and over again? This may be undesired behaviour by a user.

As I have observed, the message search is not called when Force Refresh button is pressed which is essentially the same action used by the Auto Refresh button.

@victorgawk
Copy link
Contributor Author

  1. Some pages, such as the consumer groups page, are expected to load slow if you have a large number of consumer groups. That's because we have to issue a series of Kafka requests in order to collect all the showed information. Thus, we should never abort ongoing requests, and only start the interval timer after the previous request has been finished. We should also make this behaviour clear to the user, so that they are not surprised that they don't see up-to-date values in case the requests take a long time.

Agree! I didn't think about the abort scenario (about the consumer groups page, bye PR #538 maybe? 😬).

Trying to solve the problem, should I replace the behavior:

  • call refresh every X secs

by:

  • refresh and wait until finish
  • wait X secs
  • refresh and wait until finish

And finally, to make the behavior clear to the user, add a text/span at the right side from the refresh buttons indicating the remaining time until the next refresh.

  1. We should enforce a minimum auto refresh interval of 5seconds

Current values are:

  • default: 5 sec
  • minimum: 1 sec -> 5 sec
  • maximum: 60 secs

What about the default and the maximum values?

@weeco
Copy link
Contributor

weeco commented Nov 15, 2022

about the consumer groups page, bye PR #538 maybe?

I'm afraid this is the case, I'm sorry! I acknowledged the work you put into this. If you plan to commit more features we are also always available to discuss this pre implementation via GitHub or via the Redpanda slack.

Trying to solve the problem, should I replace the behavior:

What you suggest there makes sense to me, yes please!

What about the default and the maximum values?

Default: 10 sec
Minimum: 5 sec
Maximum: ? Should we limit the maximum? 60 secs or 5min sounds reasonable anyways.

Thanks!

- Added a countdown span showing when the next refresh will happen.
- To avoid auto refresh triggering while another refresh is still ongoing, the auto refresh now will only force-refresh when no other api requests are ongoing.
- Updated auto refresh interval values:
  - default: 5 -> 10 seconds
  - minimum: 1 -> 5 seconds
  - maximum: 60 -> 300 seconds
@victorgawk
Copy link
Contributor Author

  • Added a countdown span showing when the next refresh will happen.
    • For the countdown update, the interval function was fixed with a 250 milliseconds rate in order to display the countdown decreasing at almost real time.
  • To avoid auto refresh trigger while another refresh is still running, the auto refresh timeout was changed to only trigger when no other API call is ongoing (api.activeRequests.length == 0).
  • Updated auto refresh interval values:
    • default: 5 -> 10 seconds
    • minimum: 1 -> 5 seconds
    • maximum: 60 -> 300 seconds

Visual Preview:
Animação

@weeco weeco merged commit e621e66 into redpanda-data:master Nov 22, 2022
@weeco
Copy link
Contributor

weeco commented Nov 22, 2022

Thanks @victorgawk , great first contribution :).

weeco pushed a commit that referenced this pull request Dec 1, 2022
* Add Auto Refresh Button

- Added a countdown span showing when the next refresh will happen.
- To avoid auto refresh triggering while another refresh is still ongoing, the auto refresh now will only force-refresh when no other api requests are ongoing.
- Updated auto refresh interval values:
  - default: 5 -> 10 seconds
  - minimum: 1 -> 5 seconds
  - maximum: 60 -> 300 seconds
@victorgawk victorgawk deleted the frontend/auto-refresh-button branch December 3, 2022 02:40
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