Closed robdimsdale closed 1 month ago
thanks @robdimsdale, that's exactly how I would have done it, not sure there's a "better" way. I would just avoid the code duplication and extract it to a common function.
Agreed @dlvhdr. I didn't want to tackle the refactor before being sure it was worth it. Specifically, if there was a one-liner way to achieve this in lipgloss
then refactoring into a common package wouldn't be worth it.
Can't help but imagine the idea of dealing with TTY control codes, full & half width characters, zero width characters, combining characters, breaking & non-breaking characters, etc. etc. may have been off-putting to the authors of the native code? Could spelunk it I suppose and see if they either say why, or at the very least don't do things in confusing ways eh.
Summary
Fixes an issue where labels did not wrap neatly when their total length was longer than one line. (see https://github.com/dlvhdr/gh-dash/pull/369#issuecomment-2132224396). The previous behavior was inconsistent - sometimes it would wrap the label color but not the label text; sometimes it would fail to render labels at all.
I also backported the same logic to the issue sidebar, as it was already present there but we just hadn't discovered it yet.
How did you test this change?
With a PR (and issue) that had multiple labels associated with them
Images/Videos
This is the new behavior. Notice the clean wrapping across multiple lines
Notes
The code is fairly clunky as it is manually building the multiple rows that consist of multiple lables. I couldn't find a native way to do this in the UI library (
lipgloss
). If there's a cleaner way to achieve this outcome I would prefer that!