fix: distinguish atom-mapping colours within a reaction; open compound preview below the cursor - #236
Open
VibhavSetlur wants to merge 3 commits into
Conversation
…one reaction Group colours were assigned as the first N entries of MAPPING_PALETTE, so a four-group reaction always drew the pair whose worst-case separation is only dE76 19.0 - the brown/rust versus red-orange confusion Sam reported. selectMappingColors(n) now picks, deterministically, the n-subset of the existing palette with the largest minimum pairwise perceptual distance (min dE76 over normal, protanopic and deuteranopic simulation), breaking ties on the lexicographically smallest index tuple and wrapping over the full palette when a reaction has more groups than the palette has colours. Palette hexes, group identity, group ordering and every colour consumer are unchanged; only which palette slot a group index receives moves.
The preview used placement="top", so it opened upward whenever any room existed above and was clipped at the top of the viewport. It now prefers 'bottom', flips to 'top' only when below lacks space, and adds preventOverflow with altAxis/no tether against the viewport so it can never be cut off at the top edge. Hover, focus, blur, Escape, pointer-events and ARIA wiring are unchanged.
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
1 participant
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.
Two defects Sam reported on the working reaction UI. Both are fixes only, so this is a patch release (3.5.1).
1. Atom-mapping colours within one reaction look too similar
Group colours were assigned as the first N entries of
MAPPING_PALETTE, so a four-group reaction always drew the subset whose worst-case perceptual separation is only dE76 19.0 — the brown/rust vs red-orange confusion.selectMappingColors(n)(lib/utils/atomMappingColors.ts) now picks the n-subset of the existing palette with the largest minimum pairwise perceptual distance, measured as the minimum CIE76 dE over normal, protanopic and deuteranopic simulation (lib/utils/colorDistance.ts). Deterministic: exhaustive search (C(8,n) <= 70, memoised), ties broken on the lexicographically smallest index tuple, colours emitted in ascending palette order.Worst-case separation of the colours actually used, old -> new:
No palette hex changed, so the existing colour-blind and contrast guarantees are untouched and still tested. Group identity, ordering,
groupIds, precision, bond colouring and every colour consumer are unchanged — only which palette slot a group index receives moves. Reactions with more groups than the eight-colour palette still wrap, as before; this is not a claim that every reaction is fully separable.Tritanopia is deliberately excluded from the ranking metric: the palette inherits a documented Okabe-Ito tritan limitation (
#D55E00vs#CC79A7, dE 0.96) that would flatten every subset score to ~1.2. Enlarged compound preview opens above the cursor and gets clipped
components/ui/ReactionStructureEquation.tsxusedplacement="top", so the preview opened upward whenever any room existed above, and Popper's defaultpreventOverflow(altAxis: false,tether: true) did not keep a 360px preview inside the viewport.It now prefers
bottom, flips totoponly when below lacks space, and usespreventOverflowwithaltAxis: true/tether: falseagainst the viewport, so it cannot be cut off at the top. Hover, keyboard focus, blur, Escape,pointer-events: noneand therole="tooltip"/aria-describedbywiring are unchanged.Tests
tests/unit/utils/mappingPaletteSafety.test.tskeeps its own independent colour maths and now asserts, for every group count, that the selected subset separates at least as well as the sequential one (strictly better for 2-7) and never drops below the CVD floor; plus the reported brown/rose-red collapse as a named regression case, and a parity check between the newcolorDistancemodule and that independent reference.tests/unit/components/ReactionStructureEquation.test.tsxasserts the rendered popper carriesdata-popper-placement="bottom"— verified to fail if the placement is reverted.Verification (full CI mirror, all exit 0)
npm run lint·npx tsc --noEmit·npm run test:run(34 files, 363 passed, 4 pre-existing skips) ·npm run build·npm audit --omit=dev --audit-level=high(0 vulnerabilities) ·git diff --checkBranched off current
upstream/staging. Not merged, not deployed.