fix(tags): show the tint a tag displays, in the app and in Settings - #2270
Merged
Merged
Conversation
Tags are a quiet tint of the diver's colour again, the way they looked before #2255 flooded the chip with that colour outright, and the colour swatches in Settings > Manage > Tags now offer that tint rather than the raw hex the tag is stored as. #2255 answered issue #2254, that Settings promised a colour the app never painted, by moving the app to the stored colour. The complaint is better answered the other way round: keep the quiet chips and correct what Settings promises. The tint has to become a value for that to be possible at all. A 15 per cent translucent fill is not a colour but a recipe whose result depends on the surface behind it, which is what #2254 measured: amber #F59E0B came out F7E7CF on a detail card, F2E8D7 on a dive row and C4C2B7 on a selected one, where the row's blue-grey bled through. A swatch cannot represent a display colour that does not exist. tagChipColors resolves the tint once against the theme surface and returns it opaque, so the pixel on a plain card is what it always was and the drift onto selected rows is gone. One tint, not the 0.15 of a dive row and the 0.2 of a detail card, since two tints would give a tag two display colours again. The label keeps the tag's hue and its saturation and gives up only lightness, moving away from the fill until it clears WCAG AA. The pre-#2255 label was the raw colour, about 1.7:1 on its own tint at the pale end of the palette. Darkening in HSL rather than blending towards a dark neutral is deliberate: an RGB blend compresses the channel spread and cost the saturated mid tones such as #A855F7 nearly half their saturation. The direction follows the fill, so one rule serves both themes. Every TagColorPicker swatch is now a real TagChip drawn by that same function, carrying the name as it is typed, in the Add and Edit dialogs and in the merge sheet. The Manage list row's solid CircleAvatar becomes the tag's own chip, which was the last place in the app still claiming the stored hex was what a tag looks like. The 48/32 dp close-button tap target from #2255 stays. It is an accessibility floor and has nothing to do with the fill it arrived with. Tests were written first and mutation tested: restoring the opaque fill, the translucent fill or the 0.2 tint each turns them red. Closes #2269
Contributor
|
📦 Build artifacts for this PR · commit
Artifacts expire in 7 days. Downloading requires being signed in to GitHub. macOS needs two extractions: unzip the downloaded artifact, then unzip the Updated automatically on each push. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The one line Codecov flagged as uncovered was uncovered because it only ran in a broken case. _labelOn chose which way to move the label by comparing the fill's luminance to the seed's, walked that way, exhausted its steps and fell through to a black or white that had already failed. The comparison is wrong at the ends of the range. A tag stored near black on a dark theme has a fill LIGHTER than itself, since the fill is mostly surface, so the label walked towards black, into the fill, and came out at 1.1:1. A white tag on a light theme was worse: white on white, 1.0:1. Tag.colorHex is free-form, so an imported tag can carry either. The direction is now read off the fill alone, taking whichever end of the lightness range has contrast room against it. Black clears AA for any fill above about 0.175 luminance and white for any below about 0.183, so the ranges overlap and some end always works, which makes the walk terminate by construction and retires the fallback. Output is unchanged for all twenty palette colours: the lerp is algebraically what it was, and only the choice of endpoint moved. Covered by two tests, a near-black tag on a dark surface and a near-white tag on a light one, each of which fails at about 1:1 without the fix. Patch coverage is now 100 per cent. Refs #2269
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Three moderate findings remain unresolved.
Review effort: Lite
Findings: None
What changed in this PR
This PR restores tinted tag chips and aligns Settings previews with the shared tag rendering.
Changes:
- Adds shared tint, border, and contrast-aware label calculations.
- Updates chips, color pickers, merge UI, and Manage rows.
- Expands focused tests and removes obsolete contrast helpers.
Review notes: Three moderate findings remain regarding card-surface resolution, picker touch-target height, and test scrolling.
| File | Summary |
|---|---|
test/features/tags/presentation/widgets/tag_color_picker_test.dart |
Tests picker previews and tag names. |
test/features/tags/presentation/widgets/tag_chip_test.dart |
Tests chip rendering. |
test/features/tags/presentation/tag_color_contrast_test.dart |
Removes obsolete contrast coverage. |
test/features/tags/presentation/tag_chip_colors_test.dart |
Tests tint and contrast calculations. |
test/features/tags/presentation/pages/tag_manage_page_test.dart |
Tests Manage-row rendering and interactions. |
lib/features/tags/presentation/widgets/tag_merge_sheet.dart |
Passes merge names to previews. |
lib/features/tags/presentation/widgets/tag_input_widget.dart |
Renders labelled chip swatches. |
lib/features/tags/presentation/widgets/tag_chip.dart |
Applies tinted fills and borders. |
lib/features/tags/presentation/tag_color_contrast.dart |
Removes the obsolete helper. |
lib/features/tags/presentation/tag_chip_colors.dart |
Centralizes chip color calculations. |
lib/features/tags/presentation/pages/tag_manage_page.dart |
Uses chips in Manage rows and dialogs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The swatches in Settings > Manage > Tags measured 79 by 29 on every platform, under the 48 dp touch floor and under the 32 dp pointer floor alike. The dots they replaced were 28 dp and cleared neither either, so this is an old gap rather than a new one, but it sat oddly beside a change that had just kept the chip's close button at its own floor. The 48/32 policy moves out of _DeleteButton to a shared tagTapTarget, so the close button and the swatch answer to one rule rather than two copies of it, and each swatch now reserves that box around its chip and hit tests opaquely across it. The chip itself is untouched at 29 dp: only the reachable area grew, from 177 to 272 dp of picker on touch and to 192 on a pointer, which the dialog's existing scroll view carries. Three scope tests and one merge test tapped controls that the taller picker had pushed past the fold. The merge one matters most: it asserts that an empty name does NOT merge, so a tap that missed would have passed it without ever reaching the guard. All four scroll their target into view first, and the tags suite now reports no missed taps at all. While measuring, the choice of ColorScheme.surface as the tint's fixed reference is now written down in tagChipColors. A chip on a card sits on surfaceContainerLow or higher, and tinting against whichever container the chip happens to occupy would hand back a colour per surface, which is the bug the function exists to prevent. Refs #2269
github-project-automation
Bot
moved this from In review
to Done
in Submersion Release Tracker
ericgriffin
deleted the
ericgriffin/tag-color-display-settings-ba0fcd
branch
September 23, 2026 03:06
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
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.
Closes #2269
The change
Tags are a quiet tint of the diver's colour again, the way they looked before
#2255 flooded the chip with that colour outright, and the colour swatches in
Settings > Manage > Tags now offer that tint rather than the raw hex a tag is
stored as.
#2255 answered issue #2254, that Settings promised a colour the app never
painted, by moving the app to the stored colour. The complaint is better
answered the other way round: keep the quiet chips and correct what Settings
promises.
Why the tint had to become a value
A 15 per cent translucent fill is not a colour but a recipe whose result
depends on the surface behind it. That is exactly what #2254 measured for the
amber tag
#F59E0B:F59E0B(exact)F7E7CFF2E8D7C4C2B7So a swatch cannot represent "the actual display colour" while the chip stays
translucent, because there is no single such colour to represent. The new
tagChipColorsresolves the tint once againstColorScheme.surfaceandreturns it opaque. The pixel on a plain card is what it always was; what goes
away is the drift onto selected rows, and the swatch becomes able to tell the
truth. This is the treatment
course_status_colors.dartalready uses for thecourse status card.
One tint, not the
0.15a dive row used and the0.2a detail card used:two tints would give a tag two display colours again.
The label
The label keeps the tag's hue and its saturation and gives up only lightness,
moving away from the fill until it clears WCAG AA. The pre-#2255 label was the
raw colour, which on its own 15 per cent tint is about 1.7:1 at the pale end of
the palette.
Darkening in HSL rather than blending towards a dark neutral is deliberate: an
RGB blend compresses the channel spread and cost the saturated mid tones such
as
#A855F7nearly half their saturation. The direction follows the fill, sothe one rule serves both themes, darkening on a pale chip and lightening on a
dark one.
Settings
Every
TagColorPickerswatch is now a realTagChipdrawn by that samefunction, carrying the tag's name as it is typed, in the Add and Edit dialogs
and in the merge sheet. The Manage list row's solid
CircleAvatarbecomes thetag's own chip, which was the last place in the app still claiming the stored
hex was what a tag looks like.
Twenty labelled chips wrap taller than twenty dots, so the swatches are dense
and capped in width, and the dialog's existing scroll view carries the rest.
One pre-existing dialog test now scrolls the scope editor into view before
tapping it.
What #2255 keeps
TagChipstays the single widget behind every tag, so the eight recipes itconsolidated cannot drift apart again. The 48/32 dp close-button tap target
stays too: it is an accessibility floor and has nothing to do with the fill it
arrived with.
Tests
Written first, and mutation tested. Restoring the opaque fill, the translucent
fill, or the
0.2tint each turns them red.test/features/tags/presentation/tag_chip_colors_test.dartpins the tint'schannels, pins the fill opaque, and holds every palette colour's label to AA
while proving it gives up lightness only. Hue is asserted only where there is
a hue to keep: Stone and Zinc differ by under 12 of 255 between their
strongest and weakest channel, so a one-unit rounding swings their computed
hue by degrees.
test/features/tags/presentation/widgets/tag_chip_test.dartasserts the fillis identical over a white card and over the blue-grey of a selected row,
which is the invariant the original bug broke and this change keeps.
test/features/tags/presentation/widgets/tag_color_picker_test.dartpins theswatches to the chip's colours rather than the raw hex, and to the name as it
is typed.
tag_manage_page_test.dartgains a group holding the row to a chip with nosolid dot left behind.
Full suite: 31256 passing, 31 skipped.