fix(tags): show a tag in its own colour, not a tint of it - #2255
Merged
Merged
Conversation
A tag's colour in Settings > Manage > Tags was not the colour the tag showed anywhere else. The Manage page paints the stored colour; every chip painted `tag.color.withValues(alpha: 0.15)` or `0.2` over whatever surface happened to be behind it. A translucent fill is not a colour but a recipe, so the amber tag `#F59E0B` resolved to F7E7CF on a detail card, F2E8D7 on a list row and C4C2B7 on a selected row, where the row's blue-grey bled through and the tag stopped reading as amber at all. Every tag now renders through one TagChip, filled with Tag.color at full opacity. The label colour is chosen per chip by WCAG contrast ratio, so the pale yellow and the near-black slate of the palette both stay readable. The recipe was duplicated across eight call sites, which is why each one drifted on its own; they now share the widget. The dive filter sheet and the search page keep FilterChip's selection semantics and gain the exact-colour avatar dot that the site and equipment filter sheets already used, replacing their tinted selectedColor. Closes #2254
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! |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The TagChip delete action has a 16×16 hit and semantics target and should be enlarged to at least 32×32.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR fixes inconsistent tag colors by centralizing rendering in an opaque, contrast-aware TagChip.
Changes:
- Replaces duplicated tag-chip styling across app surfaces.
- Uses WCAG-based label contrast and exact-color filter avatars.
- Updates related tests and adds rendering/contrast coverage.
| File | Summary |
|---|---|
test/features/tags/presentation/widgets/tag_chip_test.dart |
Tests chip rendering and interactions. |
test/features/tags/presentation/tag_color_contrast_test.dart |
Tests WCAG contrast. |
test/features/import_wizard/presentation/widgets/import_tags_field_test.dart |
Updates chip assertions. |
test/features/equipment/presentation/pages/equipment_edit_tags_test.dart |
Updates tag widget assertions. |
test/features/dive_sites/presentation/widgets/site_tags_card_test.dart |
Updates site tag assertions. |
test/features/dive_sites/presentation/pages/site_detail_page_test.dart |
Updates detail assertions. |
test/features/dive_log/presentation/pages/dive_edit_tag_picker_test.dart |
Updates picker assertions. |
test/features/dive_log/presentation/pages/bulk_membership_wiring_test.dart |
Updates membership assertions. |
lib/features/tags/presentation/widgets/tag_input_widget.dart |
Uses shared tag chips. |
lib/features/tags/presentation/widgets/tag_chip.dart |
Adds shared opaque tag chip; delete control needs a larger accessible target. |
lib/features/tags/presentation/tag_color_contrast.dart |
Provides contrast-based label colors. |
lib/features/import_wizard/presentation/widgets/import_tags_field.dart |
Uses unsaved tag chips. |
lib/features/equipment/presentation/widgets/equipment_tag_chips.dart |
Uses shared tag chips. |
lib/features/dive_sites/presentation/widgets/site_tags_card.dart |
Uses shared tag chips. |
lib/features/dive_sites/presentation/widgets/site_list_tile.dart |
Uses dense shared tag chips. |
lib/features/dive_log/presentation/widgets/dive_filter_sheet.dart |
Adds exact-color tag avatars. |
lib/features/dive_log/presentation/pages/dive_search_page.dart |
Adds exact-color tag avatars. |
lib/features/dive_log/presentation/pages/dive_detail_page.dart |
Uses shared tag chips. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The close button was a 16 dp icon with an InkResponse splash radius, and a splash radius does not size a hit box: the target was 16 by 16, well under either platform's floor, which put removing a tag out of reach for touch and for assistive tech. It is an IconButton again, constrained to 48 dp where a finger points and 32 dp where a mouse does, with tapTargetSize shrinkWrap so the constraints alone decide the box. A removable chip drops its vertical padding and takes its height from that target, the way a Material chip with a delete button sizes itself, so the chip does not grow to 56 dp. The size is measured per platform in the tests rather than assumed, since this is the second time a chip-sized control in this app has quietly shrunk below its own floor.
github-project-automation
Bot
moved this from In review
to Done
in Submersion Release Tracker
This was referenced Sep 22, 2026
ericgriffin
added a commit
that referenced
this pull request
Sep 23, 2026
…2270) * fix(tags): show the tint a tag displays, in the app and in Settings 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 * fix(tags): read the label's direction off the fill, not off the seed 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 * fix(tags): give a colour swatch the tap target of the control it is 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
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 #2254
The problem
A tag's colour in Settings > Manage > Tags was not the colour the tag showed
anywhere else. Measured on a light theme for "Shore", stored as
#F59E0B:F59E0B(exact)F7E7CFF2E8D7C4C2B7The data path was never at fault:
getTagsForDivesselects the whole row andmaps it through the single
mapTagRow, so the chips received the samecolorHexthe Manage page edits. The chips painted it astag.color.withValues(alpha: 0.15)(list) or0.2(cards) with no opaquebase, and a translucent fill is not a colour but a recipe: the result depends
on the surface behind it. On a selected dive row the blue-grey background bled
through and the amber tag came out grey-olive, so the same tag showed two
colours in one list.
This is the failure already documented for the course status card in
lib/features/courses/presentation/course_status_colors.dart, where atranslucent card colour let the Material 3 elevation tint composite through.
The change
TagChipis now the single widget behind every tag in the app. It fills withTag.colorat full opacity and picks its label colour by WCAG 2.1 contrastratio rather than a luminance threshold, because the crossover between black
and white sits near a luminance of 0.18 and the palette runs from a pale
yellow to a near-black slate.
Eight call sites shared the old recipe by copy, which is why each drifted on
its own. They now share the widget: the dive list rows, the dive detail Tags
card, the site tags card and site list rows, the equipment tag chips, the tag
input field, and the import wizard field (whose tags have no row yet, so they
use
TagChip.unsaved).The dive filter sheet and the dive search page keep
FilterChipand itsselection semantics. Their tinted
selectedColoris replaced by theexact-colour avatar dot that the site and equipment filter sheets already
used, so all four filter surfaces now agree.
Tests
Written first, and each one was mutation tested by reintroducing the tint:
four of them go red on it.
test/features/tags/presentation/widgets/tag_chip_test.dartpins the fillto the stored colour and asserts
Color.alphaBlend(fill, surface)is thatcolour over both a white card and the blue-grey of a selected row, which is
the invariant the bug broke.
test/features/tags/presentation/tag_color_contrast_test.dartholds everypredefined palette colour to a label above WCAG AA.
The tests that asserted on the old chip types were updated to the new one. The
full suite passes: 31076 passing, 31 skipped.