Skip to content

Conversation

@jcheng5
Copy link
Collaborator

@jcheng5 jcheng5 commented May 14, 2024

Previously, busy/idle messages were sent for each output/effect. That isn't correct, those messages are only supposed to be sent when the session as a whole becomes busy/idle.

Fixes #1379

Previously, busy/idle messages were sent for each output/effect.
That isn't correct, those messages are only supposed to be sent
when the session as a whole becomes busy/idle.
@jcheng5 jcheng5 requested a review from gadenbuie May 14, 2024 23:12
Copy link
Collaborator

@gadenbuie gadenbuie left a comment

Choose a reason for hiding this comment

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

LGTM!

@gadenbuie

This comment has been minimized.

@jcheng5
Copy link
Collaborator Author

jcheng5 commented May 15, 2024

No, they’re unrelated. This one changes the messages that are sent, #1373 has the right messages in the right order but they’re sent with the wrong timing.

@wch wch enabled auto-merge May 15, 2024 03:04
@wch
Copy link
Collaborator

wch commented May 15, 2024

Looks good to me, but we're getting playwright errors. Anyone know if they're real issues?

@jcheng5
Copy link
Collaborator Author

jcheng5 commented May 15, 2024

@wch Some are related to a missing ridgeplot dependency?? The Python 3.8 one seems to be a timeout, I can look at that in the morning.

schloerke added 2 commits May 15, 2024 09:20
…ed) from 5s to 30s

busy_indicator app takes minimum 4s to compute. Give a little bit extra time for CI to avoid adding a static wait time.
@wch wch added this pull request to the merge queue May 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 15, 2024
@schloerke schloerke merged commit 8787772 into main May 15, 2024
@schloerke schloerke deleted the busy-idle-refcount branch May 15, 2024 13:57
@schloerke
Copy link
Collaborator

Bypassing GH merge queue as the single test failure was due to a DataFrame error unrelated to busy/idle changes

schloerke added a commit that referenced this pull request May 15, 2024
* main:
  test(data frame): Verify that data frame's outputs are reset before moving forward (#1383)
  Send busy/idle at the right times (#1380)
  feat(data frame): Restore `input.<ID>_selected_rows()`. Rename `input.<ID>_data_view_indices` to `input.<ID>_data_view_rows` (#1377)
schloerke added a commit that referenced this pull request May 20, 2024
* data_view_meta:
  Add demo apps for `update_sort()` and `update_filter()`
  Add `update_sort()` and `update_filter()` to DF
  bug(output transformer): fix transformer auto-registration (#1394)
  fix: Add wait till pulse animation has started (#1393)
  bug(test-deploy): Add retries for deploy tests (#1392)
  Add busy indicator tests (#1391)
  Yield to give "synchronous" writes a chance to complete (#1388)
  chore(busy indicator): Update busy indicator css files (#1389)
  `ColumnFilter` and `ColumnSort` should use `col: num` and not `id: str` for consistency
  Lints
  Have `.data_view()` use `.data_view_info()` information for consistent subsetting
  feat(cli): Add `shiny --version` (#1387)
  fix(selectize): Accept jsonifiable values in `options` dictionary (#1382)
  bug(data frame): Use `<ID>_data_view_rows` (#1386)
  test(data frame): Verify that data frame's outputs are reset before moving forward (#1383)
  Send busy/idle at the right times (#1380)
  feat(data frame): Restore `input.<ID>_selected_rows()`. Rename `input.<ID>_data_view_indices` to `input.<ID>_data_view_rows` (#1377)
  Apply suggestions from code review
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.

Pulse busy indicator ends prematurely even when the plot hasn't finished loading

5 participants