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

Refactor Dashboard Live Poll javascript #582

Merged
merged 1 commit into from
May 1, 2022
Merged

Refactor Dashboard Live Poll javascript #582

merged 1 commit into from
May 1, 2022

Conversation

bensheldon
Copy link
Owner

@bensheldon bensheldon commented Apr 26, 2022

Follow up to #526.

  • Creates javascript class
  • Stores state in the dom as much as possible (Stimulus style)
  • Replaces #id=key replacement with data-live-poll-region={name}. This might make name collisions slightly less visible.
  • More exemplary... and makes me wonder if I should simply vendor StimulusJS to be even more exemplary.

@bensheldon bensheldon added the refactor Code changes that do not introduce new features label Apr 26, 2022
@reczy
Copy link
Contributor

reczy commented Apr 26, 2022

hey @bensheldon - since you're in the process of refactoring this...

Thoughts on programmatically toggling off live poll if a user checks a checkbox or clicks on an action button? Maybe even if search bar is dirty?

In trying to weigh possible frustrations from unexpected things happening, I'd say live poll that disrupts a user in the middle of trying to perform some action is probably a bit more annoying than not having the dashboard live update automatically.

@bensheldon
Copy link
Owner Author

Thoughts on programmatically toggling off live poll if a user checks a checkbox or clicks on an action button? Maybe even if search bar is dirty?

@reczy ooooh, that's an awesome suggestion. I can add a listener on all form input changes, and use that to toggle off polling.

Hmmm. Should polling be re-enabled on the next page load?

@reczy
Copy link
Contributor

reczy commented Apr 26, 2022

hmm... Good question. My initial gut reaction was "of course" but now that you're making me think about it, I'm not so sure.

Graphs, charts, summary details - in my mind, totally makes sense to have live poll default on. But I'm trying to think of other examples of tabular data that automatically refresh while the user is using it...

I guess tail logs, email inboxes... but both of those pretty much just add data to one end of things. The refreshes here can yield pretty wholesale/jarring changes. If you're trying to read through / analyze the table data or perform some manual action, you definitely don't want an unexpected refresh.

I think I just talked myself out of wanting default on live poll for the table.

Maybe live poll on for charts/graphs/summary data, off for the table? If you want to get fancy, maybe show a notice that says click here to show updates when something changes (I guess you could look at the good job table size or check last updated)?

Just ideas - I don't really have any strong opinions or wants here other than hoping to avoid an unexpected table refresh while I'm looking through it.

@bensheldon
Copy link
Owner Author

@reczy Thanks for thinking through that :-) We agree.

I think the use-case for current Live Poll functionality is "Put GoodJob's Dashboard on a Wallboard and passively look at it" not "GoodJob's Dashboard is reactively real-time while being operated by a developer".

Getting to "reactively real-time" will be something different, both from a design perspective (for all of the interruptive reasons you list) and technically (e.g. messaging-pushing and dom-diffing, rather than interval polling and region replacement)

@bensheldon
Copy link
Owner Author

@reczy I'm going to move your request into an Issue so it's a little easier to track. Should be simple to do now that the refactor makes the change easy 🙌

@bensheldon bensheldon merged commit 40e91bc into main May 1, 2022
@bensheldon bensheldon deleted the poll-refactor branch May 1, 2022 13:41
bkeepers added a commit to bkeepers/good_job that referenced this pull request May 12, 2022
* origin/main:
  Improve development instructions and tooling (rename bin/rails, add bin/appraisal) (bensheldon#590)
  Fix name of main branch in GH Action
  Include `ruby` version in development Gemfile; update to Ruby 3.0.4 (bensheldon#588)
  Replace test Instrumentation mocking with temporary subscriptions (bensheldon#589)
  Release good_job v2.14.2
  Reintroduce fixed "Apply to all" mass action (bensheldon#586)
  Refactor Dashboard Live Poll javascript (bensheldon#582)
  Update development dependencies (bensheldon#584)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code changes that do not introduce new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants