Skip to content

feat: highlight peaks and signals via NMRium's highlightedIds prop - #317

Draft
vcnainala wants to merge 3 commits into
developmentfrom
feat/highlight-peak-controllable
Draft

vcnainala wants to merge 3 commits into
developmentfrom
feat/highlight-peak-controllable

Conversation

@vcnainala

@vcnainala vcnainala commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Re-adds host-driven highlighting on the nmr-wrapper:action-request channel, using NMRium's public highlightedIds prop (feat: support externally controlled highlights cheminfo/nmrium#4405) instead of aliasing NMRium's private highlight module (feat: add highlightPeak action-request for host-driven peak highlighting #315, reverted in revert: remove highlightPeak bridge pending controllable NMRium API #316).
  • New actions: highlightPeak, highlightSignal, clearHighlight. Both highlight actions accept id, or nucleus + ppm (+ optional tolerance).
  • highlightSignal resolves a 1D signal by id or nucleus + delta and passes the signal id, its parent range id and the ids of peaks inside that range, so a single request lights up the multiplet, the range box, the ranges-table row and the peak.
  • When nucleus is given, the wrapper switches to that nucleus tab before highlighting.
  • highlightedIds is additive: NMRium's own hover/click highlighting keeps working, and host highlights stay until the next highlight or clearHighlight. Because host highlights never enter NMRium's internal highlight state, Delete/Backspace cannot remove the highlighted peak or range.
  • Unknown action types now emit nmr-wrapper:error instead of throwing.
  • Demo: Test load QM signals, Test highlight signal, Clear highlight buttons; fixture generated by qm-nmr-calc's payload builder (embedding-only display settings removed so panels show).

Blocked on: an NMRium release that contains cheminfo/nmrium#4405. That PR targets NMRium 3 (React 19); this repo is on React 18 with nmrium@^2.7.0, so it needs either a 2.x release with the change or a React 19 upgrade here. CI type checks fail until nmrium is bumped. Locally this was tested against a packed 2.8 build with the #4405 change applied (npm install --no-save).

Stacked on #316.

Test plan

  • npm run check-types, eslint, prettier (against the 2.8 build with #4405 applied)
  • npx playwright test --project chromium: all 10 pass, including highlight a peak, highlight a signal with its range and peak, and highlight a signal on another nucleus tab
  • Bump nmrium once released and re-run CI
  • Demo: Test load QM signals → Test highlight signal highlights the 3.69 ppm range and peak; Clear highlight clears it
  • Invalid params (no id, no nucleus + ppm) emit nmr-wrapper:error without breaking later requests

… API

Add highlightPeak, highlightSignal and clearHighlight action requests on
top of NMRium's public highlight / onHighlightChange props instead of
aliasing its private highlight module.

highlightSignal resolves a 1D signal by id or nucleus + delta and also
highlights its parent range and the peaks inside that range. Host-driven
highlights use an UNKNOWN source so NMRium's Delete/Backspace handler
cannot delete the highlighted peak or signal.
@vcnainala
vcnainala force-pushed the feat/highlight-peak-controllable branch 2 times, most recently from a076b40 to e6c826d Compare October 4, 2026 09:58
Switch from the controlled highlight / onHighlightChange API (#4402) to
the additive highlightedIds prop from cheminfo/nmrium#4405. Host
highlights no longer touch NMRium's internal highlight state, so the
sourceData workaround and the permanent option are gone.

Drop the embedding settings from the QM demo fixture: they hide NMRium's
panels, which the demo and e2e tests rely on.
@vcnainala vcnainala changed the title feat: highlight peaks and signals via NMRium's controllable highlight API feat: highlight peaks and signals via NMRium's highlightedIds prop Oct 6, 2026
Base automatically changed from revert/highlight-peak-bridge to development October 6, 2026 11:48

This branch has not been deployed

No deployments
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.

1 participant