LearnNewsExamplesServices
Frontmatter
id9367
titleFix Helix & Gallery selection models keynav and record resolution
stateClosed
labels
bugai
assigneestobiu
createdAtMar 7, 2026, 1:49 PM
updatedAtMar 7, 2026, 3:02 PM
githubUrlhttps://github.com/neomjs/neo/issues/9367
authortobiu
commentsCount1
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
blockedBy[]
blocking[]
closedAtMar 7, 2026, 3:02 PM

Fix Helix & Gallery selection models keynav and record resolution

Closed v12.1.0 bugai
tobiu
tobiu commented on Mar 7, 2026, 1:49 PM

The GalleryModel and HelixModel need to correctly resolve records when navigating via keyboard, regardless of whether useInternalId is true or false on the view.

Since store.get(id) checks both the standard map and the internalIdMap in O(1) time, we should remove the expensive fallback iterators (store.items.find(...)) inside onItemClick, onNavKeyRow, and onNavKeyColumn.

Additionally, the HelixModel.mjs select() method contains a fatal bug where it checks item.id !== itemId (where item is a string), causing it to fail to remove the neo-selected CSS class from previously selected items during navigation.

Fixes:

  1. In HelixModel and GalleryModel, rely entirely on store.get() for O(1) record resolution, removing store.items.find().
  2. Fix HelixModel.mjs select() method to handle strings correctly: change if (item.id !== itemId) to if (item !== itemId).
tobiu added the bug label on Mar 7, 2026, 1:49 PM
tobiu added the ai label on Mar 7, 2026, 1:49 PM
tobiu assigned to @tobiu on Mar 7, 2026, 1:49 PM
tobiu referenced in commit 64eaa4f - "Fix Helix & Gallery selection models keynav and record resolution (#9367) on Mar 7, 2026, 2:22 PM
tobiu
tobiu Mar 7, 2026, 2:23 PM

Input from Gemini 3.1 Pro:

✦ To elaborate on the final fix implemented here:

The core issue was a mismatch in how internalId was used. The base Helix and Gallery components have useInternalId: true by default, meaning selection models use the internalId for DOM operations and state. However, the custom app subclasses (apps/covid/... and apps/sharedcovid/...) overrode createItem and forced the DOM nodes to use the country string instead of the internalId.

This caused several cascading failures during KeyNav:

  1. Selection Failure: HelixModel tried to select neo-helix-1__neo-record-123, which didn't exist in the DOM.
  2. False Positives: The state change pushed the string 'Spain' to afterSetCountry. Because the selection model internally held 'neo-record-123', the view thought the change was "external" (e.g., from the combobox).
  3. The 1s Delay: Thinking it was an external change, it forced a re-selection and called onKeyDownSpace(null), triggering the 1000ms "animation fireworks" transition purely by accident on every keystroke.
  4. Enter Key Bug: expandItem failed to find the right node to clone because it was looking for the internalId.

The Fix:

  • Updated createItem in all 4 custom views to respect getRecordId(record) instead of hardcoding getKey().
  • Updated onSelect in the views to correctly map the internalId back to the country string, ensuring the URL hash stays clean and meaningful (#mainview=helix&country=Spain).
  • Updated afterSetCountry in the views to map the string back to the internalId before checking the selection model. This successfully restores the change-detection logic: it now correctly distinguishes between internal view-based navigation (no fireworks) and external combobox/hash changes (fireworks).
  • Added intent-driven documentation to afterSetCountry to explain this exact mechanism for future maintainers.
tobiu referenced in commit 9a87a04 - "Fix Gallery & Helix selection resilience to seamlessly support useInternalId modes on direct URL hashes (#9367)" on Mar 7, 2026, 2:40 PM
tobiu closed this issue on Mar 7, 2026, 3:02 PM