feat(#1543): adopt the DataJoint brand tier colors in diagram themes - #1544
Conversation
Retunes _DIAGRAM_THEMES to the brand palette: Imported on DataJoint Blue #00A0DF, Computed on DataJoint Orange #FF5113, brand grey Lookup, retuned Manual green; edges, cluster frames, and titles on brand navy #171C39. Dark theme derived by the existing rule (stroke hue kept, fills inverted toward navy-tinted darks, text brightened to AA on #161A21). Hue families are preserved, so tier recognition carries over; structural work from #1534 (shapes, clusters, underlines, edge weights, adaptive block) is unchanged. Two collision-free adjustments, commented in place: Part text #55585C (vs Lookup's #5A5C5F) and schema-cluster frame #171D3A (vs edge navy) so the adaptive mapping stays one-light-color-one-role. Adds test_theme_text_contrast_meets_aa (every text-on-fill pair >= 4.5:1 in both themes) and test_adaptive_mapping_is_collision_free; updates theme and mermaid expectations. Closes #1543
MilagrosMarin
left a comment
There was a problem hiding this comment.
Verified point by point against the branch and independently recomputed the color math — this is approve-ready. The palette matches the brand source of truth (datajoint-docs' extra.css: --dj-primary: #00a0df, --dj-secondary: #ff5113) and implements #1543's table exactly, with the two collision-avoidance deviations (Part text #55585C, frame #171D3A) declared and commented. All 14 text-on-fill pairs clear AA in my own WCAG computation (light min 5.52, dark min 9.27), the adaptive CSS namespaces are collision-free including the pairs outside the palette dict, and the #1534 structure is untouched as claimed.
Two non-blocking notes:
-
test_adaptive_mapping_is_collision_freehas a blind spot its invariant doesn't:mapping = {}resets inside the per-index loop, so fills (idx 0) and texts (idx 2) are validated in separate dicts — but both emit[fill="…"]selectors in_adaptive_style_block, so a future light fill equal to a light text with different dark counterparts would break the SVG while passing the test. The keys already carrykind, so hoistingmappingabove the loop unifies the namespace for free;entity_fillandschema_cluster[1](also fill rules in the adaptive block) are worth adding while there. Verified no collision exists today across the unified namespace. -
Prose nit: the light-theme range is 5.5–8.9 once
collapsed(8.93) is included — "5.5–7.8" describes the five tier rows.
…r fills and texts share the [fill=] selector space; entity_fill and cluster label included
|
Both notes addressed before merge: the collision test now runs one unified namespace per attribute kind — tier fills and texts share the |
|
@MilagrosMarin — one more approval needed: the fold-in push (your own suggested test fix, applied verbatim) reset the review under the last-push rule, and the ruleset has no admin bypass. Nothing else changed since your verification. |
MilagrosMarin
left a comment
There was a problem hiding this comment.
Recomputed every contrast pair independently and they match the body exactly: light 5.52–8.93:1, the five tier rows 5.52–7.76:1, dark 9.27–12.11:1 — all comfortably AA. Also re-ran the collision-free invariant (holds), and confirmed both one-step adjustments are load-bearing rather than cosmetic: without them Part text #5A5C5F collides with Lookup's in the fill namespace, and the cluster frame #171C39 with edge navy in the stroke namespace. Brand values line up exactly with the CSS in datajoint-docs#263.
Making the AA check part of the suite is the right move. One thing it doesn't cover: node borders. Against white, Imported #00A0DF is 2.96:1, Part #B9BBBE 1.92:1, and the default #C9BC5B 1.94:1 — under WCAG 1.4.11's 3:1 for graphical objects. Whether 1.4.11 applies here is genuinely arguable, since tier is redundantly encoded by fill, border, and shape, so the border isn't carrying the meaning alone. Raising it so it's a deliberate call rather than something noticed later.
Incidentally #00A0DF at 2.96:1 is the same figure I flagged on datajoint-docs#263 for link text — same brand constraint, but there it's body-size text where AA does apply, versus a decorative border here.
Implements #1543: the diagram tier palette anchors on the brand colors — Imported on DataJoint Blue
#00A0DF, Computed on DataJoint Orange#FF5113, brand grey Lookup, retuned Manual green — with edges, cluster frames, and titles on brand navy#171C39. The dark theme derives by the existing rule (stroke hue kept, fills inverted toward navy-tinted darks, text brightened to AA on#161A21). Hue families are preserved, so tier recognition for long-time users carries over; all structural work from #1534 (shapes, entity clusters, dimension underlines, edge weights, adaptive block) is untouched.Two one-step color adjustments keep the adaptive mapping collision-free (each light color maps to exactly one role per attribute), commented in place: Part text
#55585Cvs Lookup's#5A5C5F, and the schema-cluster frame at#171D3Avs edge navy — both visually identical to their neighbors.Tests:
test_theme_text_contrast_meets_aamakes the AA check part of the suite as #1543 asked — every text-on-fill pair ≥ 4.5:1 in both themes (light pairs measure 5.5–8.9:1 including collapsed (the five tier rows run 5.5–7.8), dark 9.3–12.1:1) — andtest_adaptive_mapping_is_collision_freelocks the #1532 invariant the adjustments protect. Theme and mermaid expectations updated (mermaid classDefs derive from the light theme, so they follow automatically; the ordinary-edge assertion moves from slate to navy). Fulltest_diagram_style.py: 10/10 against both backends locally.Downstream: docs figures re-render with the new palette on their next touch.