Skip to content

fix(favorites): show the whole route name in the route badge - #107

Merged
itousouta15 merged 1 commit into
mainfrom
fix/route-name-badge
Oct 18, 2026
Merged

itousouta15 merged 1 commit into
mainfrom
fix/route-name-badge

Conversation

@AvianJay

Copy link
Copy Markdown
Collaborator

Summary

  • The favourite route card's avatar kept only the first two characters (131 read "13", 市民小巴4延 read "市民"); search results kept four ("市民小巴").
  • Both now use a shared RouteNameBadge: the full name in the 52 px rounded square EtaBadge uses, so route and station cards line up in mixed favourite groups.
  • Names over four characters break onto two lines where a route number meets its wording (152 / 區2, 市民小巴 / 4延), at a separator (19路 / 大灣→安平) or before a bracket. Codes such as 7327D stay whole; other unbroken names are halved.
  • The badge is excluded from semantics because the tile title already reads the name.

60% of route names nationwide are five or more characters, so the badge has to handle long names rather than truncate them.

Tests

  • New test/route_name_badge_test.dart covers the line-splitting rules and the rendered badge.
  • favorites_reorder_test.dart found the card by its name text, which now also appears on the badge; it now targets the tile.
  • flutter test (527) and flutter analyze pass.

The favourite route card's avatar kept only the first two characters
(131 read "13", 市民小巴4延 read "市民"), and search results kept four
("市民小巴"). Both now use RouteNameBadge: the full name in the 52 px
rounded square EtaBadge uses, so route and station cards line up in
mixed groups.

Names over four characters break onto two lines where a route number
meets its wording (152 / 區2, 市民小巴 / 4延), at a separator
(19路 / 大灣→安平) or before a bracket; codes such as 7327D stay whole.
@itousouta15
itousouta15 self-requested a review October 18, 2026 09:56
@itousouta15
itousouta15 merged commit 1a7080f into main Oct 18, 2026
9 checks passed

@Av1anJay Av1anJay left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by Hermes Agent. Direction is right — the badge now shows the full name, the 52 px size matches EtaBadge so mixed groups line up, and ExcludeSemantics is correct because the title already reads the name. Two non-blocking issues with the splitter on real data, see inline.

Checks run

  • flutter analyze on the PR head: clean (also confirmed in the Verify job log for run 39110012988)
  • flutter test on the PR head: 527 passed (from the Verify job log)
  • Local flutter test on this box: not usable — 2 vCPU / 1 GB RAM thrashes the frontend server, so I relied on the CI result above rather than claiming a local run
  • Algorithm re-verified independently: the 17 cases in route_name_badge_test.dart all reproduce, and I ran the same split logic over 1181 distinct route names pulled from GET /api/v1/cities/{provider}/routes for all 23 providers to find the edge cases below

Findings

  • ⚠️ lib/widgets/route_name_badge.dart:48 — closing brackets aren't candidates, so 2 real routes split with ) starting line 2 and 32 names leave a bracket unclosed on line 1.
  • ⚠️ lib/widgets/route_name_badge.dart:30 — FittedBox has no floor: 148 / 1181 production names render under ~5 px (worst ~2 px).
  • 💡 Separator dropping is documented and tested, but it costs meaning in a few real names: T525(每週二、四預約) → T525(每週二 / 四預約) reads as 每週二四. Low priority.
  • ✅ Existing behaviour preserved: blank name → ?, short names whole, search result tile no longer needs its own take(4) + FittedBox wrapper, favorites_reorder_test.dart finder update is correct (the name now matches twice per tile).
  • ✅ Test coverage for the splitter and the rendered badge is good; routeBadgeLines being a pure top-level function makes it cheap to test.

Verdict: comment only — nothing blocking, and I can't approve a PR authored by the authenticated account.

}

const _lineSeparators = {' ', '-', '-', '–', '—', '/', '/', '、', '·'};
const _openingBrackets = {'(', '(', '[', '【', '〔'};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ _openingBrackets covers openers only, so nothing stops a split immediately before a closing bracket. Hand-tracing the current code on real data:

  • 0右 臺南火車站(成功路B)→臺南火車站(成功路B) — at index 13 the B→) transition counts as a code/non-code switch and sits 1 character from the middle, so it wins over the ( candidate at index 8. Result: 0右 臺南火車站(成功路B / )→臺南火車站(成功路B), i.e. line 2 opens with a stray ). Same for 0左 …(成功路A)→….
  • 32 of 1181 production route names end line 1 with an unclosed bracket, e.g. T501(每週二、四、六 發車) → T501(每週二 / 四、六 發車).

Skipping the candidate when current is a closer () ) ] 】 〕) would let these fall through to the opener instead. Worth a test case next to the 12(延駛飛機路) one.

borderRadius: BorderRadius.circular(size * 0.31),
),
child: FittedBox(
fit: BoxFit.scaleDown,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ FittedBox(scaleDown) inside a fixed 52 px box has no lower bound. Measured against real data (52 px box, 0.1 padding → 41.6 px usable, base font 17.16 px): 148 of 1181 production route names land under ~5 px effective font, and the worst cases are unreadable — 19 繞復國墩峰上承德路回不繞料羅新頭西村一 ≈ 2.2 px, 9路 9人小巴 東溪埔寮福安宮→臺南轉運站(繞公親里) ≈ 2.3 px.

It is technically "the whole name", but at 2 px it reads as a smudge, and the tile title already shows the name, so the badge contributes nothing at that size. A minimum font size (then maxLines: 2 + ellipsis, or drop the badge back to a short form) would keep it legible.

Separately: FittedBox also cancels the system text scale inside the badge — at textScaler 2.0 the text is scaled back down to fit 52 px. That's acceptable here because of ExcludeSemantics and the duplicate title, just flagging it so it isn't mistaken for a11y-safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants