fix(ui): improve timeline title spacing, truncation #3331
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.
Header title UI issues
I started this as a follow-up to #3325 because it turns out that the improved target area of the back button was clipped on the left by an
overflow: hidden
on a container, as shown here:As I looked into this, I found a few more issues with the layout of the left-side elements of the
MainContent
header (back button and/or title):There is the
overflow: hidden
issue as shown above. This can be fixed by reducing the padding on the header, removing thisoverflow: hidden
if not needed, and using padding on the children (buttons and titles) instead.Most titles are clickable and have
@click="$scrollToTop
, but their target areas are a bit small because they have no padding, meaning that if you end up clicking just a bit above, below or at the right of the text (just left of the icon) then nothing happens. If these titles are treated as clickable buttons, they should follow general usability rules for buttons. This can be fixed with a bit of padding. Here's a before and after:white-space: nowrap
, are flex children with an initial style ofmin-width: auto
, and hereauto
translates to the full width of the uninterrupted line of text. (A common fix is usingmin-width: 0
or some other small value formin-width
.)The styles for
MainContent
header titles are mostly duplicated across 20+ views. There is a sharedtimeline-title-style
style that applies a few text styles, but all the structure and spacing (especially when there is an icon) is duplicated. This makes it challenging to evolve this style to modify the spacing or fix the truncation issue.There is duplicated logic as well, because most of the instances of the
MainContent
titles use@click="$scrollToTop"
, but a few don't, and when they don't it doesn't seem to be for a specific reason, so I suspect it could be an oversight.Proposed refactoring with a new component
For all those reasons, I propose creating a shared component, tentatively named
MainTitle
(app/components/main/MainTitle.vue
), which renders a timeline title with an optional icon, with appropriate spacing and truncation, and thescrollToTop
behavior by default.This creates a pretty big diff, but it's a straightforward refactor overall. The simpler usage looks like:
When the title had an icon, we get:
Finally, half the titles doubled as links, using
NuxtLink
. This is a bit more tricky to implement using a single component. One option would be to wrap<NuxtLink><MainTitle>{{ title }}</MainTitle></NuxtLink>
, but I don't love the verbosity — or the unstyled<a>
wrapper element, which could cause styling or layout issues. I opted for this pattern, which has limitations in terms of TypeScript autocompletions for props, and requires usingRouterLink
instead ofNuxtLink
for some reasons I cannot fathom:Outsanding questions
Can
as="router-link"
be inferred from props?I tried only passing the
to
prop and then inferring the value of theis
prop (<component :is="to ? 'router-link' : 'div'">
), but it doesn't work and shows JS errors in the console.Should the title be a H1?
<span>
, a<div>
or a<a>
, with only one instance where it's a<h1>
(in one of the Settings views).router-link
(i.e. as a<a>
element) then we wouldn't have a title. This can be fixed by nesting elements a little bit, however.Do we need the primary-vs-secondary styles?
Currently the header title colors are:
var(--c-primary)
) for most views.I implemented the white style for the settings sub-pages using a
secondary?: boolean
prop, but it could make sense to drop this style and just usage orange everywhere.Should we harmonize the design of those titles and of the back button?
The icon for the header's back button and the new
<MainTitle icon="foo" />
have similar sizes and spacing, and appear in the same place.When we show either a back button or a
<MainTitle>
with an icon, things look pretty good and consistent.One possible issue is when there is both a back button and a
<MainTitle>
in the header. Both the button and the title have padding, and end up spaced pretty far apart. See the difference between thetitle_icon ↔︎ title
spacing (2 steps, i.e.0.5rem
) and theback_icon ↔︎title
spacing (2x3 steps, i.e.1.5rem
).It should be possible to harmonize both, possibly by folding the back button feature within the
<MainTitle>
component and turning it into a split button when there is both a back button and a title to display. Should we go with that, or is that too much of a change?