From bcee09280796d5a4a3f589931048d361b2220277 Mon Sep 17 00:00:00 2001 From: Alexis Janin Date: Tue, 25 Aug 2026 12:01:33 +0200 Subject: [PATCH 1/2] Make the annotation colour picker a single source of truth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 4 + src/clinical_scope/constants.py | 3 + .../dash_api/annotations/model.py | 18 +++ .../dash_api/assets/color_picker.css | 29 ++++ .../callbacks/annotation_callbacks.py | 66 ++++----- src/clinical_scope/dash_api/core_api.py | 139 ++++++++---------- src/clinical_scope/dash_api/styles.py | 40 +++++ 7 files changed, 186 insertions(+), 113 deletions(-) create mode 100644 src/clinical_scope/dash_api/assets/color_picker.css diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f787ca..c79d66a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ All notable changes to this project will be documented in this file. **What changes for you:** a configuration that already sets `trace_options` (or the Excel `trace_mode` / `line_width` / `opacity` / `marker_symbol` columns) on a device datasource starts taking effect, where before it was ignored. Where a datasource ships its own trace style, your block now wins key by key over it; keys you leave unset keep the shipped value. Nothing changes for a configuration that only styled `other::` files. +- **The annotation colour picker no longer disagrees with itself.** The row of preset swatches carried its own "selected" highlight alongside the hex code, and the two drifted apart: typing a code left the highlight behind, and opening a modal — which pre-fills the colour of the trace you clicked, rarely one of the six presets — moved the code without moving the highlight. The colour saved always came from the hex field, so the highlighted swatch was the half that lied. Presets are now plain shortcuts that fill the field, and a swatch beside it previews the colour the annotation will actually get. + + **What changes for you:** a code pasted without its leading `#` is accepted, a malformed one is flagged as you leave the field, and a colour that is not a valid six-digit hex now falls back to the default instead of being written into the annotation file as-is. + --- ## [1.1.0] — 2026-08-24 diff --git a/src/clinical_scope/constants.py b/src/clinical_scope/constants.py index ad1c061..1bb6edf 100644 --- a/src/clinical_scope/constants.py +++ b/src/clinical_scope/constants.py @@ -42,6 +42,9 @@ DEFAULT_QUICK_LOAD = False ANNOTATION_FILE_NAME = "annotations.json" ANNOTATION_KEY = "annotations" +# Doubles as the HTML `pattern` attribute of the colour fields, which is implicitly anchored — +# hence `re.fullmatch` on the Python side, so both ends accept exactly the same strings. +HEX_COLOR_PATTERN = r"#?[0-9A-Fa-f]{6}" # Signal-free, no-PHI app state cached under the user's home (~//). CLINICAL_SCOPE_DIR_NAME = ".clinical_scope" diff --git a/src/clinical_scope/dash_api/annotations/model.py b/src/clinical_scope/dash_api/annotations/model.py index cbbcf11..188154c 100644 --- a/src/clinical_scope/dash_api/annotations/model.py +++ b/src/clinical_scope/dash_api/annotations/model.py @@ -10,11 +10,14 @@ from __future__ import annotations +import re import uuid from dataclasses import dataclass, field from datetime import UTC, datetime from enum import StrEnum +import clinical_scope.constants as cst + def _now_iso() -> str: return datetime.now(tz=UTC).isoformat() @@ -37,6 +40,8 @@ class AnnotationType(StrEnum): # Preset color palette offered in the creation modal ANNOTATION_COLORS: list[str] = [ + "#999999", # gray + "#000000", # black "#e74c3c", # red "#3498db", # blue "#2ecc71", # green @@ -46,6 +51,19 @@ class AnnotationType(StrEnum): ] +def normalize_hex_color(value: str | None) -> str: + """ + Return `value` canonicalised to "#rrggbb", falling back to the first preset if malformed. + + The colour fields are free text, so a "#"-less paste is accepted and anything else malformed + resolves to the default rather than reaching annotations.json verbatim. + """ + candidate = (value or "").strip() + if re.fullmatch(cst.HEX_COLOR_PATTERN, candidate): + return f"#{candidate.lstrip('#').lower()}" + return ANNOTATION_COLORS[0] + + @dataclass class Annotation: """ diff --git a/src/clinical_scope/dash_api/assets/color_picker.css b/src/clinical_scope/dash_api/assets/color_picker.css new file mode 100644 index 0000000..2d263f7 --- /dev/null +++ b/src/clinical_scope/dash_api/assets/color_picker.css @@ -0,0 +1,29 @@ +/* ============================================================================= + COLOUR PICKER (annotation & group creation modals) + ============================================================================= */ + +/* ----------------------------------------------------------------------------- + Preset Swatches + ----------------------------------------------------------------------------- */ + +/* The presets carry no selected state, so hover is what tells the user they are clickable. */ +.color-preset-swatch:hover { + transform: scale(1.15); + box-shadow: 0 0 0 2px rgba(0, 0, 0, 0.15); +} + +.color-preset-swatch { + transition: transform 0.1s ease-in-out; +} + +/* ----------------------------------------------------------------------------- + Hex Field Validity + ----------------------------------------------------------------------------- */ + +/* The browser evaluates the input's `pattern` itself, so validity needs no callback. + `:not(:focus)` keeps it quiet mid-typing: the warning lands on blur and clears once valid. + `:has()` because the border sits on the wrapper, not on the input. */ +.hex-color-field:has(> .hex-color-input:invalid:not(:focus)) { + border-color: #e74c3c; + background-color: #fdf0ef; +} diff --git a/src/clinical_scope/dash_api/callbacks/annotation_callbacks.py b/src/clinical_scope/dash_api/callbacks/annotation_callbacks.py index cdbd180..05779ce 100644 --- a/src/clinical_scope/dash_api/callbacks/annotation_callbacks.py +++ b/src/clinical_scope/dash_api/callbacks/annotation_callbacks.py @@ -24,6 +24,7 @@ TIME_BASED_ANNOTATION_TYPES, Annotation, AnnotationType, + normalize_hex_color, ) from clinical_scope.dash_api.annotations.renderer import ( build_figure_overlays, @@ -38,6 +39,7 @@ BUTTON_ANNOTATION_INACTIVE, BUTTON_ANNOTATION_SAVE, BUTTON_MODAL_CLOSE, + COLOR_PREVIEW_SWATCH, ) from clinical_scope.datasource.formatting.timezone import to_naive_display_ts from clinical_scope.signal_container import DisplayFallbacks @@ -152,26 +154,6 @@ def _format_x_short(x_val: str | None, display_tz: str | None = None) -> str: return str(x_val) -def _build_swatch_styles(selected_color: str, swatch_ids: list[dict]) -> list[dict]: - """Return a style list for colour swatches, highlighting the selected one.""" - styles = [] - for swatch_id in swatch_ids: - color = swatch_id["color"] - border = "3px solid #333" if color == selected_color else "2px solid transparent" - styles.append( - { - "width": "22px", - "height": "22px", - "borderRadius": "50%", - "backgroundColor": color, - "cursor": "pointer", - "border": border, - "flexShrink": 0, - } - ) - return styles - - def _annotation_list_row( annotation: Annotation, group_name: str | None = None, display_tz: str | None = None ) -> html.Div: @@ -667,38 +649,52 @@ def toggle_global_checkbox_visibility(modal_data: dict) -> dict: # --------------------------------------------------------------------------- -# 5. Colour swatch pickers — one per creation modal (annotation, group) +# 5. Colour pickers — one per creation modal (annotation, group) # --------------------------------------------------------------------------- +# The hex input is the single source of truth: presets only write to it, the preview only +# reads from it. A second indicator of the selected colour would inevitably desync from it. @callback( Output("annotation-color-input", "value", allow_duplicate=True), - Output({"type": "annotation-color-swatch", "color": ALL}, "style"), Input({"type": "annotation-color-swatch", "color": ALL}, "n_clicks"), - State({"type": "annotation-color-swatch", "color": ALL}, "id"), prevent_initial_call=True, ) -def pick_annotation_color_swatch(_n_clicks_list: list, swatch_ids: list) -> tuple[str, list]: - """Highlight the selected colour swatch and update the annotation modal hex input.""" +def pick_annotation_color_swatch(_n_clicks_list: list) -> str: + """Write the clicked preset into the annotation modal hex input.""" if ctx.triggered_id is None: raise PreventUpdate - selected = ctx.triggered_id["color"] - return selected, _build_swatch_styles(selected, swatch_ids) + return ctx.triggered_id["color"] @callback( Output("group-color-input", "value", allow_duplicate=True), - Output({"type": "group-color-swatch", "color": ALL}, "style"), Input({"type": "group-color-swatch", "color": ALL}, "n_clicks"), - State({"type": "group-color-swatch", "color": ALL}, "id"), prevent_initial_call=True, ) -def pick_group_color_swatch(_n_clicks_list: list, swatch_ids: list) -> tuple[str, list]: - """Highlight the selected colour swatch and update the group modal hex input.""" +def pick_group_color_swatch(_n_clicks_list: list) -> str: + """Write the clicked preset into the group modal hex input.""" if ctx.triggered_id is None: raise PreventUpdate - selected = ctx.triggered_id["color"] - return selected, _build_swatch_styles(selected, swatch_ids) + return ctx.triggered_id["color"] + + +@callback( + Output("annotation-color-preview", "style"), + Input("annotation-color-input", "value"), +) +def update_annotation_color_preview(color: str) -> dict: + """Mirror the annotation hex input, showing the colour that Create would actually save.""" + return {**COLOR_PREVIEW_SWATCH, "backgroundColor": normalize_hex_color(color)} + + +@callback( + Output("group-color-preview", "style"), + Input("group-color-input", "value"), +) +def update_group_color_preview(color: str) -> dict: + """Mirror the group hex input, showing the colour that the group would actually get.""" + return {**COLOR_PREVIEW_SWATCH, "backgroundColor": normalize_hex_color(color)} # --------------------------------------------------------------------------- @@ -735,7 +731,7 @@ def create_annotation( annotation_type = AnnotationType(modal_data["type"]) is_global = "global" in (global_checkbox or []) subplot_name = None if is_global else modal_data.get("subplot_name") - color = color or ANNOTATION_COLORS[0] + color = normalize_hex_color(color) if annotation_type == AnnotationType.TIME_EVENT: data = {"x": modal_data["x"], "xaxis": modal_data.get("xaxis", "x")} @@ -1363,7 +1359,7 @@ def activate_group( if triggered_id == "create-group-btn": if not name: raise PreventUpdate - color = color or ANNOTATION_COLORS[0] + color = normalize_hex_color(color) annotation_type = AnnotationType(annotation_type_value or AnnotationType.TIME_EVENT.value) is_global = ( "global" in (scope_value or []) and annotation_type in TIME_BASED_ANNOTATION_TYPES diff --git a/src/clinical_scope/dash_api/core_api.py b/src/clinical_scope/dash_api/core_api.py index a4651b1..d9e92ec 100644 --- a/src/clinical_scope/dash_api/core_api.py +++ b/src/clinical_scope/dash_api/core_api.py @@ -37,6 +37,10 @@ BUTTON_PROCESS, BUTTON_RELOAD, BUTTON_UPLOAD, + COLOR_HEX_FIELD, + COLOR_HEX_INPUT, + COLOR_PRESET_SWATCH, + COLOR_PREVIEW_SWATCH, COLOR_PURPLE, INSPECTION_MODAL_HEADER_ROW, INSPECTION_MODAL_PANEL, @@ -196,29 +200,58 @@ ], ) + # --------------------------------------------------------------------------- -# Annotation creation modal +# Colour picker — shared by both creation modals # --------------------------------------------------------------------------- -_color_swatches = html.Div( - [ - html.Div( - id={"type": "annotation-color-swatch", "color": color}, - n_clicks=0, - style={ - "width": "22px", - "height": "22px", - "borderRadius": "50%", - "backgroundColor": color, - "cursor": "pointer", - "border": "2px solid transparent", - "flexShrink": 0, - }, - ) - for color in ANNOTATION_COLORS - ], - style={"display": "flex", "gap": "6px", "alignItems": "center"}, -) +def _color_picker(swatch_type: str, input_id: str, preview_id: str) -> html.Div: + """ + Build a colour picker: preset shortcuts alongside the hex field holding the chosen colour. + The hex field holds the colour; presets write into it and the preview swatch mirrors it. + Malformed input is flagged by the field's own `pattern`, styled in color_picker.css. + """ + return html.Div( + [ + html.Div( + [ + html.Div( + id={"type": swatch_type, "color": color}, + n_clicks=0, + className="color-preset-swatch", + style={**COLOR_PRESET_SWATCH, "backgroundColor": color}, + ) + for color in ANNOTATION_COLORS + ], + style={"display": "flex", "gap": "6px", "alignItems": "center"}, + ), + html.Div( + [ + html.Div( + id=preview_id, + style={**COLOR_PREVIEW_SWATCH, "backgroundColor": ANNOTATION_COLORS[0]}, + ), + dcc.Input( + id=input_id, + type="text", + value=ANNOTATION_COLORS[0], + maxLength=7, + pattern=cst.HEX_COLOR_PATTERN, + className="hex-color-input", + style=COLOR_HEX_INPUT, + ), + ], + className="hex-color-field", + style=COLOR_HEX_FIELD, + ), + ], + style={"display": "flex", "alignItems": "center", "gap": "10px"}, + ) + + +# --------------------------------------------------------------------------- +# Annotation creation modal +# --------------------------------------------------------------------------- _annotation_modal = html.Div( id="annotation-modal", style=ANNOTATION_MODAL_STYLE_HIDDEN, @@ -284,25 +317,10 @@ "Color", style={"fontSize": "13px", "fontWeight": "bold", "marginBottom": "4px"}, ), - html.Div( - [ - _color_swatches, - dcc.Input( - id="annotation-color-input", - type="text", - value=ANNOTATION_COLORS[0], - maxLength=7, - style={ - "width": "90px", - "padding": "4px 8px", - "border": "1px solid #ced4da", - "borderRadius": "4px", - "fontSize": "12px", - "fontFamily": "monospace", - }, - ), - ], - style={"display": "flex", "alignItems": "center", "gap": "10px"}, + _color_picker( + "annotation-color-swatch", + "annotation-color-input", + "annotation-color-preview", ), ], style={"marginBottom": "12px"}, @@ -353,26 +371,6 @@ # --------------------------------------------------------------------------- # Annotation group creation modal # --------------------------------------------------------------------------- -_group_color_swatches = html.Div( - [ - html.Div( - id={"type": "group-color-swatch", "color": color}, - n_clicks=0, - style={ - "width": "22px", - "height": "22px", - "borderRadius": "50%", - "backgroundColor": color, - "cursor": "pointer", - "border": "2px solid transparent", - "flexShrink": 0, - }, - ) - for color in ANNOTATION_COLORS - ], - style={"display": "flex", "gap": "6px", "alignItems": "center"}, -) - _annotation_group_modal = html.Div( id="annotation-group-modal", style=ANNOTATION_MODAL_STYLE_HIDDEN, @@ -456,25 +454,10 @@ "Color", style={"fontSize": "13px", "fontWeight": "bold", "marginBottom": "4px"}, ), - html.Div( - [ - _group_color_swatches, - dcc.Input( - id="group-color-input", - type="text", - value=ANNOTATION_COLORS[0], - maxLength=7, - style={ - "width": "90px", - "padding": "4px 8px", - "border": "1px solid #ced4da", - "borderRadius": "4px", - "fontSize": "12px", - "fontFamily": "monospace", - }, - ), - ], - style={"display": "flex", "alignItems": "center", "gap": "10px"}, + _color_picker( + "group-color-swatch", + "group-color-input", + "group-color-preview" ), ], style={"marginBottom": "12px"}, diff --git a/src/clinical_scope/dash_api/styles.py b/src/clinical_scope/dash_api/styles.py index ba265a7..ef6ad74 100644 --- a/src/clinical_scope/dash_api/styles.py +++ b/src/clinical_scope/dash_api/styles.py @@ -309,3 +309,43 @@ "borderBottom": "1px solid #f0f0f0", "fontSize": "13px", } + +# --------------------------------------------------------------------------- +# Colour picker (annotation & group creation modals) +# --------------------------------------------------------------------------- + +# No "selected" variant by design: the hex field alone records the chosen colour. +COLOR_PRESET_SWATCH: dict = { + "width": "22px", + "height": "22px", + "borderRadius": "50%", + "cursor": "pointer", + "flexShrink": 0, +} + +# Square and inset in the hex field, so it reads as that field's value and not a seventh preset. +COLOR_PREVIEW_SWATCH: dict = { + "width": "22px", + "height": "22px", + "borderRadius": "2px", + "margin": "3px", + "flexShrink": 0, +} + +# Wrapper carrying the border, so the preview swatch and the text sit in one visual control. +COLOR_HEX_FIELD: dict = { + "display": "flex", + "alignItems": "center", + "border": "1px solid #ced4da", + "borderRadius": "4px", + "overflow": "hidden", +} + +COLOR_HEX_INPUT: dict = { + "width": "80px", + "padding": "4px 8px 4px 2px", + "border": "none", + "outline": "none", + "fontSize": "12px", + "fontFamily": "monospace", +} From e0dd8bd8da5393fc9982ebc8180374f1b4edbcaf Mon Sep 17 00:00:00 2001 From: Alexis Janin Date: Tue, 25 Aug 2026 12:03:48 +0200 Subject: [PATCH 2/2] ruff format --- src/clinical_scope/dash_api/core_api.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/clinical_scope/dash_api/core_api.py b/src/clinical_scope/dash_api/core_api.py index d9e92ec..da7539e 100644 --- a/src/clinical_scope/dash_api/core_api.py +++ b/src/clinical_scope/dash_api/core_api.py @@ -455,9 +455,7 @@ def _color_picker(swatch_type: str, input_id: str, preview_id: str) -> html.Div: style={"fontSize": "13px", "fontWeight": "bold", "marginBottom": "4px"}, ), _color_picker( - "group-color-swatch", - "group-color-input", - "group-color-preview" + "group-color-swatch", "group-color-input", "group-color-preview" ), ], style={"marginBottom": "12px"},