Frontmatter
| id | 9367 |
| title | Fix Helix & Gallery selection models keynav and record resolution |
| state | Closed |
| labels | bugai |
| assignees | tobiu |
| createdAt | Mar 7, 2026, 1:49 PM |
| updatedAt | Mar 7, 2026, 3:02 PM |
| githubUrl | https://github.com/neomjs/neo/issues/9367 |
| author | tobiu |
| commentsCount | 1 |
| parentIssue | null |
| subIssues | [] |
| subIssuesCompleted | 0 |
| subIssuesTotal | 0 |
| blockedBy | [] |
| blocking | [] |
| closedAt | Mar 7, 2026, 3:02 PM |
Fix Helix & Gallery selection models keynav and record resolution
tobiu assigned to @tobiu on Mar 7, 2026, 1:49 PM

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
internalIdwas used. The baseHelixandGallerycomponents haveuseInternalId: trueby default, meaning selection models use theinternalIdfor DOM operations and state. However, the custom app subclasses (apps/covid/...andapps/sharedcovid/...) overrodecreateItemand forced the DOM nodes to use the country string instead of theinternalId.This caused several cascading failures during KeyNav:
- Selection Failure:
HelixModeltried to selectneo-helix-1__neo-record-123, which didn't exist in the DOM.- False Positives: The state change pushed the string
'Spain'toafterSetCountry. Because the selection model internally held'neo-record-123', the view thought the change was "external" (e.g., from the combobox).- 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.- Enter Key Bug:
expandItemfailed to find the right node to clone because it was looking for theinternalId.The Fix:
- Updated
createItemin all 4 custom views to respectgetRecordId(record)instead of hardcodinggetKey().- Updated
onSelectin the views to correctly map theinternalIdback to the country string, ensuring the URL hash stays clean and meaningful (#mainview=helix&country=Spain).- Updated
afterSetCountryin the views to map the string back to theinternalIdbefore 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
afterSetCountryto explain this exact mechanism for future maintainers.
tobiu closed this issue on Mar 7, 2026, 3:02 PM
The
GalleryModelandHelixModelneed to correctly resolve records when navigating via keyboard, regardless of whetheruseInternalIdistrueorfalseon the view.Since
store.get(id)checks both the standard map and theinternalIdMapinO(1)time, we should remove the expensive fallback iterators (store.items.find(...)) insideonItemClick,onNavKeyRow, andonNavKeyColumn.Additionally, the
HelixModel.mjsselect()method contains a fatal bug where it checksitem.id !== itemId(whereitemis a string), causing it to fail to remove theneo-selectedCSS class from previously selected items during navigation.Fixes:
HelixModelandGalleryModel, rely entirely onstore.get()for O(1) record resolution, removingstore.items.find().HelixModel.mjsselect()method to handle strings correctly: changeif (item.id !== itemId)toif (item !== itemId).