Lazy-load concept tabs (Visualization, Mappings, Notes) - #538
Open
matthewhorridge wants to merge 4 commits into
Open
Lazy-load concept tabs (Visualization, Mappings, Notes)#538matthewhorridge wants to merge 4 commits into
matthewhorridge wants to merge 4 commits into
Conversation
The Visualization tab's `<iframe>` had a hardcoded `src`, so it loaded eagerly the moment the concept view rendered — even while the tab was hidden — on every class select. Loading BioMixer kicks off a large fan-out of backend calls, so this put avoidable load on the server for a tab that is often never viewed. The partial had an `original_src` / `@immediate_load` deferral, but it was never functional: nothing ever set `@immediate_load`, and no JS swapped the iframe's src, so `original_src` was always the loading spinner and the hardcoded `src` is what actually made the tab work (eagerly). Replace it with a real load-on-open: render the iframe with no `src` (the BioMixer URL is held in a data attribute), and add a small `biomixer-lazy` Stimulus controller that sets the `src` when the Visualization tab is opened — either immediately if that pane is already active on render, or on the first `tab-selected` event for it. It loads once and isn't re-fetched on subsequent tab switches. Note on pane detection: the concept tab-panes are nested inside the ontology-level tab-panes, so the controller checks the NEAREST enclosing `.tab-pane`'s active state, not any active ancestor. Verified: on the Details tab the iframe has no `src` (not loaded); opening Visualization sets it and loads; switching away and back does not reload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Mappings tab fetched its expensive `get_concept_table` endpoint TWICE on every class select, both while the tab was hidden: once for the tab body (a turbo-frame with a bare `src`, which loads on insertion) and once for the "(N)" count shown in the tab label (a separate lazy frame that, being in the always-visible tab bar, loaded immediately). That endpoint runs single_class(full) + get_concept_mappings, so this was a large amount of wasted work for a tab that is often never opened. - Make the table lazy: add `loading: 'lazy'` to the body turbo-frame. Inactive concept panes are display:none, so the frame defers its fetch until the tab is shown. - Drop the tab-label count: the label is now plain "Mappings". Removing it eliminates the second, eager fetch entirely — the endpoint is no longer hit until the user actually opens the tab. Remove the now-unused concept_mappings_loader helper and the .concepts-mapping-count styles that only supported that count. (The mapping-creation flow's turbo_stream still targets `mapping_count`; that update now no-ops harmlessly since the frame is gone, and the table update is unaffected.) Verified: on the Details tab, get_concept_table fires 0 times (was 2); opening the Mappings tab fires it once and renders the table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Notes tab rendered inline in the concept_show response, and the controller fetched @notes (concept.explore.notes) on every class select — both to show the "(N)" count in the tab label and to populate the (hidden) Notes pane. So the notes were fetched on every select even when the tab was never opened. Unlike Mappings/Visualization, Notes had no on-demand loader, so add one: - New concepts#notes_list action + /ajax/concepts/notes_list route that fetches @notes and renders the notes list wrapped in a `concept_notes` turbo-frame (new concepts/notes_list.html.haml, same pattern as the Mappings response). - The Notes tab body is now a `loading: 'lazy'` turbo-frame pointing at that route, so it fetches only when the tab is opened (inactive panes are display:none). - Drop the "(N)" count from the tab label (now plain "Notes") and remove the @notes fetch from the show action — nothing else in that flow needs it. (notes_controller.js still bumps #note_count when a note is added; that element is gone now, so the line no-ops harmlessly — the new note row is still added.) Verified: on the Details tab, notes_list fires 0 times (was fetched every select); opening the Notes tab fires it once and renders the list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dropping the Mappings tab-label count left the `mapping_count` turbo-frame with no consumer: the shared _concept_mappings partial still emitted it, and the mapping-create turbo_stream still updated it (a no-op, since the frame no longer exists in the page). Remove both — the frame's only purpose was feeding that tab-label count. The Mappings tab (concept_mappings frame) and the create flow's table refresh are unaffected: verified the tab still loads and renders the table, and the create stream still updates concept_mappings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
|
@matthewhorridge - I merged your PRs 531 and 536 to master. This branch is now stale and conflicts with master. Could you rebase before I review this? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #537.
Loads the concept view's non-active tabs lazily, so selecting a class no longer does
work for tabs the user may never open. Previously, every class select eagerly loaded
the Visualization iframe, fetched the Mappings endpoint twice (tab body + the label
count), and fetched the Notes list — all while those tabs were hidden.
Changes
biomixer-lazyStimulus controller sets the iframesrcon tab-select (or immediatelyif that pane is already active on render). The partial had a deferral mechanism
(
original_src/@immediate_load), but it was never wired up, so the hardcodedsrcloaded eagerly.
loading: 'lazy', and the "(N)" tab-labelcount is removed. The count was a second, eager fetch of the same expensive
get_concept_tableendpoint. Removed the now-deadmapping_countturbo-frame and itscreate-flow stream update.
(
concepts#notes_listaction +/ajax/concepts/notes_listroute), and the "(N)"count is removed. The eager
@notesfetch is dropped fromconcepts#show.Details is left eager (it's the default active tab and server-cheap).
Effect
Per class select on the Details tab, the previously-wasted fetches (Visualization iframe,
Mappings ×2, Notes) no longer fire — they load only when their tab is opened. See #537 for
the measurements (cold-cache timings over 100+ deep classes) behind this.
Notes
lightweight count-only fetch could add them back if they turn out to be wanted.
notes_controller.jsstill bumps#note_countwhen a note is added; that element isgone now, so the line no-ops harmlessly (the new note row is still added).
Verified in-browser for each tab: nothing fetches on the Details view; opening a tab loads
and renders it.