Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions lib/features/tags/presentation/pages/tag_manage_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'package:submersion/features/tags/domain/entities/tag.dart';
import 'package:submersion/features/tags/presentation/providers/tag_providers.dart';
import 'package:submersion/features/tags/presentation/tag_scope_labels.dart';
import 'package:submersion/features/tags/presentation/tag_usage_messages.dart';
import 'package:submersion/features/tags/presentation/widgets/tag_chip.dart';
import 'package:submersion/features/tags/presentation/widgets/tag_input_widget.dart';
import 'package:submersion/features/tags/presentation/widgets/tag_merge_sheet.dart';
import 'package:submersion/l10n/l10n_extension.dart';
Expand Down Expand Up @@ -226,13 +227,22 @@ class _TagManagePageState extends ConsumerState<TagManagePage> {
final isSelected = _selectedIds.contains(tag.id);

return ListTile(
// Nothing but the checkbox column: the tag itself is the title, since
// a chip carries the name and the colour together (issue #2269). A
// solid dot here was the last place in the app still claiming the
// stored hex was what a tag looks like.
leading: SelectionLeading(
isSelectionMode: _isSelectionMode,
isChecked: isSelected,
onChanged: (_) => _toggleSelection(tag.id),
child: CircleAvatar(radius: 16, backgroundColor: tag.color),
child: const SizedBox.shrink(),
),
minLeadingWidth: 0,
horizontalTitleGap: 0,
title: Align(
alignment: AlignmentDirectional.centerStart,
child: TagChip(tag: tag),
),
title: Text(tag.name),
// Where the tag is offered (issues #1765, #1942).
subtitle: Text(
[
Expand Down Expand Up @@ -303,6 +313,9 @@ class _TagManagePageState extends ConsumerState<TagManagePage> {
const SizedBox(height: 8),
TagColorPicker(
selectedColor: selectedColor,
// Each swatch previews this tag by name, so the colour
// being chosen is the colour that will be seen.
nameController: controller,
onColorSelected: (color) =>
setDialogState(() => selectedColor = color),
),
Expand Down Expand Up @@ -395,6 +408,9 @@ class _TagManagePageState extends ConsumerState<TagManagePage> {
const SizedBox(height: 8),
TagColorPicker(
selectedColor: selectedColor,
// Each swatch previews this tag by name, so the colour
// being chosen is the colour that will be seen.
nameController: controller,
onColorSelected: (color) =>
setDialogState(() => selectedColor = color),
),
Expand Down
112 changes: 112 additions & 0 deletions lib/features/tags/presentation/tag_chip_colors.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import 'package:flutter/material.dart';

/// The three colours a tag chip is painted with (issue #2269).
typedef TagChipColors = ({Color fill, Color border, Color label});

/// How much of the tag's colour the fill carries.
///
/// One value, not the 0.15 a dive row used and the 0.2 a detail card used:
/// two tints give a tag two display colours, and a swatch in Settings can
/// only tell the truth about one of them.
const double _fillTint = 0.15;

/// WCAG 2.1 AA for body text.
const double _minLabelContrast = 4.5;

/// How finely the label's lightness is searched. Enough steps that a colour
/// moves no further than it has to.
const int _labelSteps = 20;

/// The colours a tag chip paints for a tag whose stored colour is [seed].
///
/// A tag chip is a quiet tint of the diver's colour rather than a chip
/// flooded with it, which is how tags looked before #2255 and how they look
/// again. The difference is that the tint is resolved here, once, against the
/// theme's own surface and returned opaque.
///
/// That matters beyond tidiness. A translucent fill is not a colour but a
/// recipe whose result depends on whatever is painted behind it, so the same
/// tag used to come out four different colours across the app and a fifth on
/// a selected row, where the row's blue-grey bled through (issue #2254).
/// Resolving the tint here means a tag has exactly one display colour, which
/// is what lets the Settings swatches offer it.
///
/// [ColorScheme.surface] is a deliberate fixed reference, not an oversight:
/// a chip on a card sits on `surfaceContainerLow` or higher, and tinting
/// against whichever container the chip happens to be in would hand back a
/// different colour per surface, which is the bug this exists to prevent. The
/// chip is meant to read as a chip against its background rather than as a
/// wash of it, and the full-strength border carries the identity either way.
/// `course_status_colors.dart` names its own surface for the same reason, and
/// names the one its card actually uses because that card has only one home.
TagChipColors tagChipColors(BuildContext context, Color seed) =>
tagChipColorsFor(
seed: seed,
surface: Theme.of(context).colorScheme.surface,
);

/// [tagChipColors] without a [BuildContext], for callers that already hold
/// the scheme and for tests that need to name the surface they are checking.
TagChipColors tagChipColorsFor({required Color seed, required Color surface}) {
final fill = Color.alphaBlend(seed.withValues(alpha: _fillTint), surface);
return (fill: fill, border: seed, label: _labelOn(fill, seed));
}

/// The label colour for a chip filled with [fill] by a tag coloured [seed].
///
/// The pre-#2255 chips wrote the label in the raw [seed], which on its own
/// 15 per cent tint is about 1.7:1 for the pale end of the palette: legible
/// in theory, invisible in practice. #2255 answered that with black or white,
/// which reads but drops the tag's identity from the text.
///
/// The seed instead keeps its hue and its saturation and gives up only
/// lightness, moving away from the fill until it clears AA. Working in HSL
/// rather than blending towards a dark neutral matters: an RGB blend
/// compresses the channel spread, which cost the saturated mid tones such as
/// `#A855F7` nearly half their saturation on the way down.
///
/// The direction is read off the fill alone, so the one rule serves both
/// themes: the label darkens on a pale chip and lightens on a dark one.
///
/// It is read off the fill rather than off the fill against the seed, which
/// is a distinction that only shows up at the ends of the range. A tag stored
/// near black on a dark theme has a fill LIGHTER than itself, because the
/// fill is mostly surface; comparing the two then walked the label towards
/// black, into the fill, and left it at 1.1:1. A white tag on a light theme
/// was worse still, white on white at 1.0:1.
Color _labelOn(Color fill, Color seed) {
if (tagContrastRatio(seed, fill) >= _minLabelContrast) return seed;

final hsl = HSLColor.fromColor(seed);

// Whichever end of the lightness range has room against this fill. Black
// clears AA for any fill above about 0.175 luminance and white for any
// below about 0.183, so the two ranges overlap: some end always works,
// whatever the fill, and the walk below always terminates.
final endpoint =
tagContrastRatio(Colors.black, fill) >=
tagContrastRatio(Colors.white, fill)
? 0.0
: 1.0;

for (var step = 1; step < _labelSteps; step++) {
final t = step / _labelSteps;
final candidate = hsl
.withLightness(hsl.lightness + (endpoint - hsl.lightness) * t)
.toColor();
if (tagContrastRatio(candidate, fill) >= _minLabelContrast) {
return candidate;
}
}
return hsl.withLightness(endpoint).toColor();
}

/// WCAG 2.1 contrast ratio between two opaque colours, from 1 (identical) to
/// 21 (black on white).
double tagContrastRatio(Color a, Color b) {
final la = a.computeLuminance();
final lb = b.computeLuminance();
final lighter = la > lb ? la : lb;
final darker = la > lb ? lb : la;
return (lighter + 0.05) / (darker + 0.05);
}
30 changes: 0 additions & 30 deletions lib/features/tags/presentation/tag_color_contrast.dart

This file was deleted.

62 changes: 35 additions & 27 deletions lib/features/tags/presentation/widgets/tag_chip.dart
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
import 'package:flutter/material.dart';

import 'package:submersion/features/tags/domain/entities/tag.dart';
import 'package:submersion/features/tags/presentation/tag_color_contrast.dart';
import 'package:submersion/features/tags/presentation/tag_chip_colors.dart';

/// A tag shown in the colour the diver gave it (issue #2254).
/// A tag shown in the colour the diver gave it (issues #2254, #2269).
///
/// Every tag chip in the app renders through this widget, so a tag looks the
/// same in a dive list row, on a detail card, beside a site and beside a
/// piece of equipment. The fill is [Tag.color] at full opacity, never a
/// translucent tint: a tint is not a colour but a recipe, and the chips that
/// used one resolved to a different colour on every surface, turning an amber
/// tag grey-olive on a selected row. The label takes the colour that
/// contrasts with the fill, since the palette spans a pale yellow and a
/// near-black slate.
/// piece of equipment. The chip is a quiet tint of [Tag.color] outlined in
/// that colour, which is how a tag looked before #2255 flooded the chip with
/// the colour outright.
///
/// The tint is a value, not a translucent overlay: [tagChipColors] resolves
/// it against the theme surface and hands back an opaque fill. A translucent
/// fill is a recipe rather than a colour, and the chips that used one came
/// out differently on every surface, turning an amber tag grey-olive on a
/// selected row.
class TagChip extends StatelessWidget {
TagChip({
super.key,
Expand Down Expand Up @@ -40,7 +43,8 @@ class TagChip extends StatelessWidget {

final String name;

/// The fill, painted opaque. [Tag.color] for a stored tag.
/// The tag's own colour, which the outline shows at full strength and the
/// fill shows as a tint. [Tag.color] for a stored tag.
final Color color;

/// Tapping the chip, usually to open what carries the tag.
Expand All @@ -58,16 +62,21 @@ class TagChip extends StatelessWidget {

@override
Widget build(BuildContext context) {
final foreground = tagForegroundColor(color);
final colors = tagChipColors(context, color);
final borderRadius = BorderRadius.circular(dense ? 4 : 8);
final textStyle = Theme.of(context).textTheme.bodySmall?.copyWith(
color: foreground,
color: colors.label,
fontSize: dense ? 11 : null,
);

Widget chip = Material(
color: color,
borderRadius: borderRadius,
color: colors.fill,
// The outline is the one place the tag's colour is shown at full
// strength, which is what keeps a pale tint identifiable.
shape: RoundedRectangleBorder(
borderRadius: borderRadius,
side: BorderSide(color: colors.border),
),
child: InkWell(
onTap: onTap,
borderRadius: borderRadius,
Expand Down Expand Up @@ -98,7 +107,7 @@ class TagChip extends StatelessWidget {
if (onDeleted != null) ...[
const SizedBox(width: 4),
_DeleteButton(
color: foreground,
color: colors.label,
// Material's own chips label their delete button from the
// same string, so a caller that has nothing better to say
// keeps the wording screen readers already know.
Expand All @@ -121,6 +130,17 @@ class TagChip extends StatelessWidget {
}
}

/// The floor for a tag control's tap target.
///
/// A finger needs the 48 dp Material touch minimum; a pointer is precise, and
/// a 48 dp box inside a chip is out of scale on a desktop, so it takes the
/// 32 dp pointer minimum instead. Shared by the chip's close button and by
/// the colour swatches in Settings, which are chips the diver taps.
double tagTapTarget(TargetPlatform platform) => switch (platform) {
TargetPlatform.android || TargetPlatform.iOS || TargetPlatform.fuchsia => 48,
TargetPlatform.macOS || TargetPlatform.linux || TargetPlatform.windows => 32,
};

/// The close button of a removable chip.
///
/// The tap target is measured, not assumed: a bare icon with a splash radius
Expand All @@ -138,21 +158,9 @@ class _DeleteButton extends StatelessWidget {
final VoidCallback onPressed;
final String? tooltip;

/// The floor for the tap target. A finger needs the 48 dp Material touch
/// minimum; a pointer is precise, and a 48 dp box inside a chip is out of
/// scale on a desktop, so it takes the 32 dp pointer minimum instead.
static double targetFor(TargetPlatform platform) => switch (platform) {
TargetPlatform.android ||
TargetPlatform.iOS ||
TargetPlatform.fuchsia => 48,
TargetPlatform.macOS ||
TargetPlatform.linux ||
TargetPlatform.windows => 32,
};

@override
Widget build(BuildContext context) {
final target = targetFor(Theme.of(context).platform);
final target = tagTapTarget(Theme.of(context).platform);
return IconButton(
onPressed: onPressed,
tooltip: tooltip,
Expand Down
Loading
Loading