Skip to content

Preserve the active concept view when selecting a class (#533) - #536

Merged
jvendetti merged 2 commits into
ncbo:masterfrom
matthewhorridge:fix/533-preserve-active-pane-on-class-select
Jul 30, 2026
Merged

Preserve the active concept view when selecting a class (#533)#536
jvendetti merged 2 commits into
ncbo:masterfrom
matthewhorridge:fix/533-preserve-active-pane-on-class-select

Conversation

@matthewhorridge

Copy link
Copy Markdown

Summary

On an ontology's Classes page, selecting a different class in the left-hand tree
reset the right-hand view back to Details, even when another view
(Visualization / Notes / Mappings) was open. This preserves the active view across
class selections. Fixes #533.

Root cause

Selecting a class reloads the concept_show Turbo frame, and
concepts/_show.html.haml hard-coded selected: true on the Details tab — so
every reload rebuilt the pane with Details active, regardless of the previously
open view. The active view wasn't tracked anywhere.

What changed

Persist the active view in a view URL param and read it back server-side:

  • Give the concept tabs url_parameter: 'view' so selecting a view records it.
  • Drive each tab's selected: from selected_concept_view? (defaults to
    details) instead of hard-coding Details.
  • Carry the current view param into the concept_show frame request on class
    select, via a document-level turbo:before-fetch-request hook (race-free
    compared with rewriting the link href; registered once, stateless, idempotent).

Guarding against a blank pane

Each tab decides on its own whether it's active — its selected: is
selected_concept_view?('<its id>'), i.e. "is the current view my id?". Nothing
guarantees that some tab matches. So if the view param is a value no tab has
(a typo, a stale link from before a view was renamed, a hand-edited URL), then
every tab's check returns false, no tab is marked active, and the right-hand pane
renders completely blank — no active tab, no content.

To prevent that, current_concept_view doesn't return the raw param. It validates
it against the known view ids (CONCEPT_VIEWS) and returns details whenever the
param is missing or not one of them. So an unrecognised ?view=… degrades to
the Details view instead of a blank pane, and only a genuinely valid value selects
a non-default view.

TabsContainerComponent gains an opt-in merge_url_params flag rather than
changing URL behaviour globally:

  • merge (concept views): keep the other query params — notably conceptid
    when the view changes.
  • replace (default; top-level p sections, submission section tabs):
    switching a tab still yields a clean single-param URL with no stale leftovers.

The flag is emitted as an explicit "true"/"false" string because Rails renders
a boolean data value as a valueless attribute, which the controller would
otherwise misread.

The view is now deep-linkable — e.g. ?view=concept-mappings opens Mappings
directly.

Tests

Adds test/helpers/ontologies_helper_test.rb covering the default / valid / bogus
/ blank param cases, and asserting CONCEPT_VIEWS stays in sync with the tab ids
rendered by concepts/_show.html.haml (so adding/renaming a view can't silently
ship an unselectable tab).

Verification

Verified manually in a browser:

  • Visualization / Notes / Mappings each survive a class change (URL keeps p +
    conceptid + view).
  • Default (no param) and a bogus ?view= both fall back to Details.
  • Deep links open the right view.
  • Top-level p section tabs still produce a clean ?p=… with no stale params.

Coverage caveat / follow-up

There's no automated browser/system test for the Turbo + Stimulus round-trip
(the literal issue behaviour). The test suite doesn't currently run in the dev
environment — it's missing the test config (config/bioportal_config_test.rb), so
bin/rails test won't start, and there's no Capybara system-test setup. Rather
than add a browser test that can't be run and confirmed here, the gap is logged as
a deliberate follow-up: add a "select Mappings → click another class → Mappings
stays active" system test once the test environment is runnable.

Upstream

The same bug is present in ontoportal/ontoportal_web_ui (also tracked at
ontoportal#75). This patch doesn't port cleanly — upstream's
concept tabs have no id:, include an extra instances tab, and build their
json_links differently — so an adapted upstream PR is a separate follow-up
rather than part of this one.

🤖 Generated with Claude Code

Selecting a different class in the tree reloads the concept_show turbo-frame, and
_show.html.haml hard-coded selected:true on the Details tab, so the view always
reset to Details regardless of which view (Visualization / Notes / Mappings) was
open.

Persist the active view in a 'view' URL param and read it back server-side:
- give the concept tabs url_parameter: 'view' so clicking a tab records it;
- drive each tab's selected: from selected_concept_view? (defaults to details)
  instead of hard-coding Details;
- carry the current 'view' param into the concept_show frame request on class
  select, via a document-level turbo:before-fetch-request hook (race-free vs
  rewriting the href; registered once, stateless and idempotent).

current_concept_view validates the param against the known view ids (CONCEPT_VIEWS)
and falls back to details when it is missing OR unrecognised, so a stale/typo'd
?view=... doesn't leave every tab deselected and the pane blank.

TabsContainerComponent gains an opt-in merge_url_params flag rather than changing
URL behaviour globally:
- merge (concept views): keep the other query params, notably conceptid, when the
  view changes;
- replace (default; top-level p sections, submission section tabs): switching a tab
  still yields a clean single-param URL with no stale leftovers.
The flag is emitted as an explicit "true"/"false" string because Rails renders a
boolean data value as a valueless attribute, which the controller would misread.

Bonus: the view is now deep-linkable (?view=concept-mappings opens Mappings).

Adds OntologiesHelperTest covering the default / valid / bogus / blank param cases
and asserting CONCEPT_VIEWS stays in sync with the tab ids in _show.html.haml.

Verified in-browser: Visualization/Notes/Mappings survive a class change (URL keeps
p+conceptid+view); default and bogus view fall back to Details; deep links open the
right view; top-level p tabs produce a clean ?p=... with no stale params.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jvendetti jvendetti self-assigned this Jul 30, 2026
ontology_object_tabs_component is shared with ontology_object_details_component
(schemes, collections), so hardcoding url_parameter/merge_url_params made those
pages write ?view=Details on a Details click. Make them arguments defaulting to
no URL change, and opt in from concepts/_show.html.haml.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jvendetti

Copy link
Copy Markdown
Member

@matthewhorridge - thanks for this - I reviewed and pushed one commit to the branch.

ontology_object_tabs_component has a second caller: ontology_object_details_component, which backs schemes/_show.html.haml and collections/_show.html.haml.

Hardcoding url_parameter: 'view', merge_url_params: true in the helper applied to those pages too, and since their Details tab passes no id:, TabItemComponent#id falls back to the translated title, so clicking it on ?p=schemes pushed ?p=schemes&view=Details where it previously left the URL alone.

The commit makes url_parameter: / merge_url_params: arguments defaulting to nil / false, with concepts/_show.html.haml opting in explicitly (schemes and collections revert to their previous behaviour).

I checked by hand that Details on ?p=schemes and ?p=collections leaves the URL alone, and that view persistence across class selection, deep links, and the bogus-param fallback all still work.

@jvendetti
jvendetti merged commit 780b75a into ncbo:master Jul 30, 2026
2 checks passed
@matthewhorridge

Copy link
Copy Markdown
Author

Many thanks @jvendetti

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.

Selecting a class in the tree always forces the Details pane, overriding the current pane

2 participants