Skip to content

Make the annotation colour picker a single source of truth - #87

Merged
AlexisJanin merged 2 commits into
mainfrom
bug-color-picker-inconsistent-behavior
Aug 25, 2026
Merged

Make the annotation colour picker a single source of truth#87
AlexisJanin merged 2 commits into
mainfrom
bug-color-picker-inconsistent-behavior

Conversation

@AlexisJanin

Copy link
Copy Markdown
Collaborator

Reported from use: the annotation colour picker's "selected" swatch did not always match the colour applied, nor the hex code shown next to it.

The bug

Both creation modals showed the chosen colour twice — a highlighted preset swatch and a hex field — kept in sync only by the swatch-click callback, which wrote both. Three other paths moved one side and left the other stale:

Path Moves Leaves stale
Typing in the hex field field swatch highlight
Opening the annotation modal (update_modal_ui) field swatch highlight
Opening the group modal (open_group_modal) field swatch highlight

The colour actually saved was always read from the hex field (State on annotation-color-input / group-color-input), never from swatch state — so the highlight was the half that lied.

It was also worse than "occasionally wrong". The annotation modal pre-fills suggested_color, which is the clicked trace's line colour and almost never one of the six presets, so the highlight pointed at the wrong swatch on most openings.

The fix

Rather than re-syncing two indicators more carefully, remove the second one. The hex field is now the only place a colour lives.

  • Presets become plain shortcuts. They write into the field and nothing else. _build_swatch_styles and the style Output on both pick callbacks are deleted, so no second indicator remains to desync. update_modal_ui and open_group_modal needed no change — they already set the field, and the preview follows it.
  • A preview swatch mirrors the field, showing the colour that would actually be saved. Square and inset inside the field's border, so it reads as that field's value rather than a seventh preset.
  • Validation moved out of the callback layer and onto the field's own pattern attribute, styled by a CSS :invalid rule. :not(:focus) keeps it quiet mid-typing and clears it the moment the value is valid — neither of which a blur-triggered callback would have given.
  • Submit-time normalisation (normalize_hex_color) closes a data-integrity gap: previously only an empty field fell back, so a non-empty invalid string was written to annotations.json verbatim. A code pasted without its leading # is now accepted instead of discarded.
  • The two byte-identical layout blocks collapse into one _color_picker builder; shared literals moved to constants.py and styles.py per the repo's constants rule.

Net effect on the callback layer: the design this replaced would have needed 4 new callbacks (live preview + blur-gated border, x2 modals). This needs 2, and core_api.py loses 138 lines of duplication.

Behaviour change

A colour code that is not a valid six-digit hex now resolves to the default instead of being saved as-is. Recorded in the changelog under [Unreleased].

Review notes

  • No test churn: nothing in tests/ exercised _build_swatch_styles, either pick callback, or the colour fallbacks — verified before editing.
  • Build: core_api.spec bundles the whole assets directory, so the new color_picker.css ships with no spec change.
  • One known risk: the invalid-state border uses CSS :has(). If it ever fails, the degradation is silent and graceful — the border stays grey and the preview swatch still shows the mismatch.

Worth eyeballing in the running app: open either modal, type banana and tab out (border red, preview red), then type 1abc9c without the # (border clears, preview teal). Then open the annotation modal from a trace click — the field should pre-fill that trace's colour with the preview matching, the case that was previously always wrong.

🤖 Generated with Claude Code

alexisj-inria and others added 2 commits August 25, 2026 12:01
Both creation modals showed the chosen colour twice — a highlighted preset
swatch and a hex field — and only the swatch-click callback wrote both. Every
other path moved one and left the other stale: typing in the field, and either
modal opening, which resets the field alone. The annotation modal pre-fills the
clicked trace's colour, almost never one of the six presets, so the highlight
was wrong on most openings rather than occasionally.

The colour actually saved always came from the hex field. The highlight was the
half that lied.

- Make the hex field the only place the colour lives. Presets become plain
  shortcuts that write into it; `_build_swatch_styles` and the style Output on
  both pick callbacks are gone, so there is no second indicator left to desync.
  `update_modal_ui` and `open_group_modal` needed no change — they already set
  the field, and the preview now follows it.
- Add a preview swatch mirroring what would be saved, square and inset in the
  field's border so it does not read as a seventh preset.
- Validate through the field's own `pattern` attribute and a CSS `:invalid`
  rule rather than a callback. `:not(:focus)` keeps it quiet mid-typing and it
  clears the moment the value is valid, neither of which a blur-triggered
  callback would have given.
- Normalise at submit in `normalize_hex_color`, so `annotations.json` can no
  longer receive a non-hex string: previously only an empty field fell back and
  anything else was written verbatim. A code pasted without its `#` is now
  accepted rather than discarded.
- Collapse the two byte-identical layout blocks into one `_color_picker`
  builder, with the shared literals moved to `constants.py` and `styles.py`.

Behaviour change: a colour code that is not a valid six-digit hex resolves to
the default instead of being saved as-is. Recorded in the changelog.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AlexisJanin
AlexisJanin merged commit d2b8bfd into main Aug 25, 2026
3 checks passed
@AlexisJanin
AlexisJanin deleted the bug-color-picker-inconsistent-behavior branch August 25, 2026 10:11
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.

2 participants