-
Notifications
You must be signed in to change notification settings - Fork 26
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
Conversation
- 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.
- 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`.
This reverts commit ab25353.
…shroomObserver/mushroom-observer into jdc-2319-stimulus-inat-import
This reverts commit 135b5ac.
This reverts commit e10607c.
- Separates call to Stimulus controller from data replaces by Stimulus
Add `module` arg to make it hit the namespaced controller `InatImports::JobTrackersController`
- Change text that was copied from FieldSlipJobTrackers
- InatImport#show now needs a `tracker_id` param - Fixes failure in prior commit
- Don't need ivars because it redirects instead of rendering
Should have been done in #6ecd082
- That's where FieldSlipJobTrackersController#show is rendering it. - Adds `id` to affected element.
- The request throws an error.
Although this PR changes only a tiny part of the display, I want to get it merged:
|
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.
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 |
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.
@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.
<p> | ||
<button class="btn btn-default" onclick="location.reload();"> | ||
<%= :inat_import_tracker_refresh.l %> | ||
</button> | ||
</p> |
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.
@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.)
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.
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.
Thanks!
I think I posted that after you approved the PR.
Understood about getting the page to a place where a full refresh is
unneeded.
(The full refresh will go away after I figure out how to update everything
via Stimulus & Turbo.)
…On Tue, Jan 14, 2025 at 1:36 PM andrew nimmo ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In app/views/controllers/inat_imports/show.html.erb
<#2586 (comment)>
:
> + <p>
+ <button class="btn btn-default" onclick="location.reload();">
+ <%= :inat_import_tracker_refresh.l %>
+ </button>
+ </p>
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
— that's the idea of Turbo.
—
Reply to this email directly, view it on GitHub
<#2586 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAALDFB74ALHWDBTELPRH2L2KV7NZAVCNFSM6AAAAABTZRWQCWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNJRGA3DOOBTGU>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***
.com>
|
Status
line of the InatImports#show page via Stimulus & Turbo.This is the first step in dynamically the entire page.