Changed all map lables to look the same across all map ui#206
Conversation
…alignment and styling
…athTraceMap screens for improved alignment
There was a problem hiding this comment.
Pull request overview
This PR updates the styling/layout of map marker labels to make them visually consistent across the various map screens, and adjusts the self marker presentation on the main map screen.
Changes:
- Standardize the marker label bubble layout (offset, padding, alignment, text style) across multiple map UIs.
- Update main map’s self marker to use an icon and label it as the localized “You”.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| lib/screens/path_trace_map.dart | Updates node label marker layout/styling to match the new shared look. |
| lib/screens/map_screen.dart | Updates self marker content + standardizes node label marker layout/styling. |
| lib/screens/line_of_sight_map_screen.dart | Aligns endpoint label bubble styling with the new shared look. |
| lib/screens/channel_message_path_screen.dart | Updates hop/node label marker layout/styling to match the new shared look. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| child: FittedBox( | ||
| fit: BoxFit.contain, | ||
| child: Container( | ||
| padding: const EdgeInsets.symmetric( | ||
| horizontal: 6, | ||
| vertical: 2, | ||
| ), | ||
| decoration: BoxDecoration( | ||
| color: Colors.black54, | ||
| borderRadius: BorderRadius.circular(8), | ||
| ), | ||
| alignment: Alignment.center, | ||
| child: Text( | ||
| endpoint.label, | ||
| maxLines: 1, | ||
| overflow: TextOverflow.ellipsis, | ||
| style: const TextStyle( | ||
| color: Colors.white, | ||
| fontSize: 11, | ||
| fontWeight: FontWeight.w500, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Using FittedBox(fit: BoxFit.contain) here prevents TextOverflow.ellipsis from taking effect (the text is laid out unconstrained and then scaled). That will shrink long endpoint labels instead of truncating them, which can hurt readability. Consider constraining the label width and using BoxFit.scaleDown (or removing FittedBox) if ellipsis is desired.
| child: FittedBox( | |
| fit: BoxFit.contain, | |
| child: Container( | |
| padding: const EdgeInsets.symmetric( | |
| horizontal: 6, | |
| vertical: 2, | |
| ), | |
| decoration: BoxDecoration( | |
| color: Colors.black54, | |
| borderRadius: BorderRadius.circular(8), | |
| ), | |
| alignment: Alignment.center, | |
| child: Text( | |
| endpoint.label, | |
| maxLines: 1, | |
| overflow: TextOverflow.ellipsis, | |
| style: const TextStyle( | |
| color: Colors.white, | |
| fontSize: 11, | |
| fontWeight: FontWeight.w500, | |
| ), | |
| ), | |
| child: Container( | |
| padding: const EdgeInsets.symmetric( | |
| horizontal: 6, | |
| vertical: 2, | |
| ), | |
| decoration: BoxDecoration( | |
| color: Colors.black54, | |
| borderRadius: BorderRadius.circular(8), | |
| ), | |
| alignment: Alignment.center, | |
| child: Text( | |
| endpoint.label, | |
| maxLines: 1, | |
| overflow: TextOverflow.ellipsis, | |
| style: const TextStyle( | |
| color: Colors.white, | |
| fontSize: 11, | |
| fontWeight: FontWeight.w500, | |
| ), |
| child: FittedBox( | ||
| fit: BoxFit.contain, | ||
| child: Container( | ||
| padding: const EdgeInsets.symmetric(horizontal: 6, vertical: 2), | ||
| decoration: BoxDecoration( | ||
| color: Colors.black54, | ||
| borderRadius: BorderRadius.circular(8), | ||
| ), | ||
| alignment: Alignment.center, | ||
| child: Text( | ||
| label, | ||
| maxLines: 1, | ||
| overflow: TextOverflow.ellipsis, | ||
| style: const TextStyle( | ||
| color: Colors.white, | ||
| fontSize: 11, | ||
| fontWeight: FontWeight.w500, |
There was a problem hiding this comment.
FittedBox(fit: BoxFit.contain) causes the label content to scale to fit rather than applying TextOverflow.ellipsis (since the child is laid out with unbounded width). This can make long labels very small. If you want consistent text sizing with truncation, add a width constraint and use BoxFit.scaleDown or remove the FittedBox.
| child: FittedBox( | ||
| fit: BoxFit.contain, | ||
| child: Container( | ||
| padding: const EdgeInsets.symmetric(horizontal: 6, vertical: 2), | ||
| decoration: BoxDecoration( | ||
| color: Colors.black54, | ||
| borderRadius: BorderRadius.circular(8), | ||
| ), | ||
| alignment: Alignment.center, | ||
| child: Text( | ||
| label, | ||
| maxLines: 1, | ||
| overflow: TextOverflow.ellipsis, | ||
| style: const TextStyle( | ||
| color: Colors.white, | ||
| fontSize: 11, | ||
| fontWeight: FontWeight.w500, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Wrapping the label Container in a FittedBox with fit: BoxFit.contain means the child is laid out with essentially unconstrained width, so TextOverflow.ellipsis will never trigger; long labels will be scaled down instead (potentially to unreadable sizes), and short labels can be scaled up. If the intent is truncation with ellipsis, constrain the text width (e.g., via SizedBox/ConstrainedBox) and use BoxFit.scaleDown (or drop FittedBox entirely).
| child: FittedBox( | |
| fit: BoxFit.contain, | |
| child: Container( | |
| padding: const EdgeInsets.symmetric(horizontal: 6, vertical: 2), | |
| decoration: BoxDecoration( | |
| color: Colors.black54, | |
| borderRadius: BorderRadius.circular(8), | |
| ), | |
| alignment: Alignment.center, | |
| child: Text( | |
| label, | |
| maxLines: 1, | |
| overflow: TextOverflow.ellipsis, | |
| style: const TextStyle( | |
| color: Colors.white, | |
| fontSize: 11, | |
| fontWeight: FontWeight.w500, | |
| ), | |
| ), | |
| child: Container( | |
| padding: const EdgeInsets.symmetric(horizontal: 6, vertical: 2), | |
| decoration: BoxDecoration( | |
| color: Colors.black54, | |
| borderRadius: BorderRadius.circular(8), | |
| ), | |
| alignment: Alignment.center, | |
| child: Text( | |
| label, | |
| maxLines: 1, | |
| overflow: TextOverflow.ellipsis, | |
| style: const TextStyle( | |
| color: Colors.white, | |
| fontSize: 11, | |
| fontWeight: FontWeight.w500, | |
| ), |
| child: FittedBox( | ||
| fit: BoxFit.contain, | ||
| child: Container( | ||
| padding: const EdgeInsets.symmetric(horizontal: 6, vertical: 2), | ||
| decoration: BoxDecoration( | ||
| color: Colors.black54, | ||
| borderRadius: BorderRadius.circular(8), | ||
| ), | ||
| alignment: Alignment.center, | ||
| child: Text( | ||
| label, | ||
| maxLines: 1, | ||
| overflow: TextOverflow.ellipsis, | ||
| style: const TextStyle( | ||
| color: Colors.white, | ||
| fontSize: 11, | ||
| fontWeight: FontWeight.w500, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
FittedBox(fit: BoxFit.contain) will scale the label up/down to fill the marker bounds, and because the child is laid out unconstrained, TextOverflow.ellipsis won’t actually apply. This can make long labels tiny rather than ellipsized. Consider restoring a width constraint and using BoxFit.scaleDown (or removing FittedBox) so truncation behaves as intended.
| child: FittedBox( | |
| fit: BoxFit.contain, | |
| child: Container( | |
| padding: const EdgeInsets.symmetric(horizontal: 6, vertical: 2), | |
| decoration: BoxDecoration( | |
| color: Colors.black54, | |
| borderRadius: BorderRadius.circular(8), | |
| ), | |
| alignment: Alignment.center, | |
| child: Text( | |
| label, | |
| maxLines: 1, | |
| overflow: TextOverflow.ellipsis, | |
| style: const TextStyle( | |
| color: Colors.white, | |
| fontSize: 11, | |
| fontWeight: FontWeight.w500, | |
| ), | |
| ), | |
| child: Container( | |
| padding: const EdgeInsets.symmetric(horizontal: 6, vertical: 2), | |
| decoration: BoxDecoration( | |
| color: Colors.black54, | |
| borderRadius: BorderRadius.circular(8), | |
| ), | |
| alignment: Alignment.center, | |
| child: Text( | |
| label, | |
| maxLines: 1, | |
| overflow: TextOverflow.ellipsis, | |
| style: const TextStyle( | |
| color: Colors.white, | |
| fontSize: 11, | |
| fontWeight: FontWeight.w500, | |
| ), |
| child: const Icon( | ||
| Icons.person_pin_circle, | ||
| color: Colors.white, | ||
| size: 20, |
There was a problem hiding this comment.
The self marker switched from a localized "You" text to an icon. Other map UIs in this PR (e.g., path_trace_map/channel_message_path_screen) still render "You" text inside the self marker, so the self marker style is now inconsistent across screens. If the goal is uniformity, either update the other screens to use the same icon or keep the text here.
| child: const Icon( | |
| Icons.person_pin_circle, | |
| color: Colors.white, | |
| size: 20, | |
| child: Text( | |
| context.l10n.pathTrace_you, | |
| textAlign: TextAlign.center, | |
| style: const TextStyle( | |
| color: Colors.white, | |
| fontWeight: FontWeight.bold, | |
| fontSize: 12, | |
| ), |
| offset: const Offset(0, -20), | ||
| child: FittedBox( | ||
| fit: BoxFit.contain, | ||
| child: Container( | ||
| padding: const EdgeInsets.symmetric(horizontal: 6, vertical: 2), | ||
| decoration: BoxDecoration( | ||
| color: Colors.black54, | ||
| borderRadius: BorderRadius.circular(8), | ||
| ), | ||
| alignment: Alignment.center, |
There was a problem hiding this comment.
This _buildNodeLabelMarker implementation is duplicated across multiple screens (map_screen, path_trace_map, channel_message_path_screen, line_of_sight_map_screen). Since changes need to be kept in sync, it would be less error-prone to extract this into a shared widget/helper (e.g., a MapMarkerLabel widget) used by all screens.
Changed all map lables to look the same across all map ui