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

Inat Import Stimulus Status #2586

Merged
merged 40 commits into from
Jan 14, 2025
Merged

Inat Import Stimulus Status #2586

merged 40 commits into from
Jan 14, 2025

Conversation

JoeCohen
Copy link
Member

@JoeCohen JoeCohen commented Dec 18, 2024

  • Dynamically updates the Status line of the InatImports#show page via Stimulus & Turbo.
  • Uses js (instead of a Rails request) for a manual refresh of the entire page.

This is the first step in dynamically the entire page.

JoeCohen and others added 11 commits December 14, 2024 15:53
- Uses Stimulus controller to update stats 1/second
- **Does not work**; element on page doesn't update, and there's an infinite loop
- `InatImportJobTracker.status` returns a tring describing the status, e.g. "Done"
rather than an ordinal.
- Misc changes to comments
Write turbo response and view
Also change routes
…ter receiving Turbo response

It's using the value of the innerHTML of the span for its `status`. This could be a data attribute of a replaced element, if we were replacing rather than updating.
@JoeCohen JoeCohen closed this Dec 18, 2024
@JoeCohen JoeCohen deleted the jdc-2319-stimulus-inat-import branch December 18, 2024 13:31
@JoeCohen JoeCohen restored the jdc-2319-stimulus-inat-import branch December 18, 2024 13:36
@JoeCohen JoeCohen reopened this Dec 18, 2024
@coveralls
Copy link
Collaborator

coveralls commented Dec 18, 2024

Coverage Status

coverage: 93.446% (-0.06%) from 93.502%
when pulling c224b36 on jdc-2319-stimulus-inat-import
into 9b5b651 on main.

@JoeCohen JoeCohen linked an issue Dec 18, 2024 that may be closed by this pull request
17 tasks
JoeCohen and others added 11 commits December 18, 2024 09:37
- Conforms it to new inat_imports_controller
- Moves get("inat_imports/authorization_response")
ahead of the resource routes
(For reasons I don't understand `http://localhost:3000/inat_imports/authorization_response` was being routed to `InatImports#show`.
- Separates call to Stimulus controller from data replaces by Stimulus
Add `module` arg to make it hit the namespaced controller `InatImports::JobTrackersController`
@JoeCohen JoeCohen marked this pull request as ready for review January 14, 2025 19:53
@JoeCohen
Copy link
Member Author

Although this PR changes only a tiny part of the display, I want to get it merged:

@JoeCohen JoeCohen added the iNat interaction with iNat label Jan 14, 2025
Copy link
Contributor

@nimmolo nimmolo left a comment

Choose a reason for hiding this comment

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

Seems good to go. If you're merging soon, maybe you can deploy my minor PR #2653 at the same time.

tag.span(id: "status", class: "text-right",
data: { controller: "inat-import-job",
endpoint: inat_import_job_tracker_path(@inat_import, @tracker) }) do
# Inner span is replaced by Stimulus
Copy link
Contributor

Choose a reason for hiding this comment

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

@JoeCohen - this inner span (below) would be your entire partial. You would of course call the partial here too, I should have written it that way.

The outer span, that stays in the main template, is what has the "data-controller" attribute, establishing the "domain" of the Stimulus controller. So we dont want that in the partial- that's why we split this into two spans. If it were in the partial, Stimulus would disconnect and reconnect each second. You want a stable connection.

The inner span has data attributes too. It has a "data-inat-import-job-target" attribute purely for convenience. Designating something as a target in Stimulus allows you to refer to the element without trying to "find it" with CSS selectors, as you would normally have to do in Javascript. You can just say this.inatImportTarget and access or change its properties directly. Not really a big saver in this case because it's the only child element, unlike the autocompleters which have dozens. But a good way to learn about targets.

The other data attribute on the inner span is the data-status. That's just where you're stashing the updated job status when the partial is updated. Your Stimulus controller would be written to check this attribute before "polling" Rails for a new status (partial) update, and if it's completed it would stop polling.

Comment on lines +41 to +45
<p>
<button class="btn btn-default" onclick="location.reload();">
<%= :inat_import_tracker_refresh.l %>
</button>
</p>
Copy link
Member Author

Choose a reason for hiding this comment

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

@nimmolo Is this the idiomatic way to do this? (It was suggested by AI, and it worked.)
(Previously was a Rails request, but that threw Errors. JS worked. This particular code was suggested by ChatGPT.)

Copy link
Contributor

@nimmolo nimmolo Jan 14, 2025

Choose a reason for hiding this comment

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

Missed this. We have a helper called js_button that I would maybe use here.

But generally speaking, the JS just refreshes the page, exactly like using the browser refresh button.
If you're writing Stimulus to refresh the elements on the page, you'd want to get your page code a place where you don't need a full page refresh, or this button — that's the idea of Turbo.

@JoeCohen
Copy link
Member Author

@nimmolo Thanks for the comments, explanation, and approval.
I will merge and deploy this and your previously merged #2653

@JoeCohen JoeCohen changed the title Inat Import Stimulus Inat Import Stimulus Status Jan 14, 2025
@JoeCohen JoeCohen merged commit 5921b83 into main Jan 14, 2025
6 of 8 checks passed
@JoeCohen
Copy link
Member Author

JoeCohen commented Jan 14, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iNat interaction with iNat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Stimulus to update JobTracker Status
3 participants