withastro / starlight

🌟 Build beautiful, accessible, high-performance documentation websites with Astro
https://starlight.astro.build
MIT License
5.05k stars 532 forks source link

Last update date position #417

Closed HiDeoo closed 1 year ago

HiDeoo commented 1 year ago

What version of starlight are you using?

0.5.6

What version of astro are you using?

2.9.3

What package manager are you using?

pnpm

What operating system are you using?

Mac

What browser are you using?

Chrome

Describe the Bug

In some docs, we have the last update date feature enabled as it is relevant to the content documented. Most of the pages are a mix of manually created pages and auto-generated pages. The auto-generated pages have their editUrl set to false in the frontmatter.

When navigating between a manually created page and an auto-generated page, the last update position is switching from the right side to the left side of the page due to the edit link being hidden. I think this is not the best user experience to have identical elements being placed in different positions between pages.

For example, on a potentially manually created page with an edit link:

SCR-20230727-kfxr

And the next page which is potentially auto-generated and has the edit link hidden:

SCR-20230727-kfvk

If this is something judged as a potential issue, I guess a potential fix could be to use a grid (1fr 1fr) and always assign the edit link to the first column and the last update date to the second column.

Note: the repro is a bit useless as I don't think it's possible to have last update date working on Stackblitz.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-b3fbut?file=src%2Fcontent%2Fdocs%2Freference%2Fexample.md

Participation

delucis commented 1 year ago

Ah, yeah. This is working “as intended” on the basis that I found it looked a bit odd to have the date right-aligned with nothing on the left. But I only really considered sites where this would be consistent across pages — i.e. edit link disabled, so last updated is always on the left. I hadn’t considered the case where this can vary page by page.

Maybe “date is always at inline-end” makes sense given that variation?

lorenzolewis commented 1 year ago

I’d +1 for consistency on this one and to have it always on the trailing edge.

HiDeoo commented 1 year ago

Ah, yeah. This is working “as intended” on the basis that I found it looked a bit odd to have the date right-aligned with nothing on the left. But I only really considered sites where this would be consistent across pages — i.e. edit link disabled, so last updated is always on the left.

That makes total sense.

Just for clarity, inline-end in the last sentence is a way to describe having the date always at the end, right? It's not a CSS property / value that I may not know about?

lorenzolewis commented 1 year ago

This one you might need to switch the footer over to display: grid instead of flex: https://github.com/withastro/starlight/blob/c7b2a4e9c8c55564be75f0c0901e38577ac764ec/packages/starlight/components/Footer.astro#L24

Then set justify-self: end on the trailing component: https://css-tricks.com/snippets/css/complete-guide-grid/#aa-justify-self.

Only reason it would have to be switched to grid is because display: flex; isn't affected by justify-self.

lorenzolewis commented 1 year ago

Or could nest the conditional <EditLink> within a span (with the condition) so that the span is still there even if not rendered, then I don't think you'd have to change any styling (not sure if the same would need to be done for <LastUpdated> for rtl languages or not).

HiDeoo commented 1 year ago

Yeah, you are right, switching to a grid was also my initial idea that I described in the issue as I think it would be the easiest way to achieve this but this inline-end made me wonder if there was another way I didn't know about ^^

Personally, I think I would prefer updating the style and avoid an empty tag but this is a solution too.

lorenzolewis commented 1 year ago

Ah, I missed the part about specifically assigning the items to their relevant column 🤓

delucis commented 1 year ago

Quick sketch of the different permutations and layout as desired if I’ve understood correctly.

So on the left, in wide enough layouts for both edit link and last updated date, the desire is to have the date always aligned to the end of the line. On the right, in narrower viewports, these items wrap, and should probably always align with the start of the line as they do currently?

sketch showing different layout combinations of edit link and last updated date

Here’s the current behaviour for comparison. Only the wide viewport variant with just the updated date is different:

sketch showing different layout combinations of edit link and last updated date

Without wishing to discourage anyone from exploring solutions, I’m stumped for how to do this. We rely on flex-wrap so that these items naturally wrap to two lines when they don’t fit (because we don’t reliably know their content: different dates, different languages, etc.), so can’t just throw a breakpoint at it. Happy to let people explore options though!

HiDeoo commented 1 year ago

I think the sketch covers all cases indeed.

On the right, in narrower viewports, these items wrap, and should probably always align with the start of the line as they do currently?

This makes total sense imo.

Happy to let people explore options though!

I'll try to play with it a little bit but indeed like you mentioned it may be a little tricky to get all these requirements.

HiDeoo commented 1 year ago

As expected, I did not manage to find a solution with all these requirements either.

Considering having a mix between pages with an edit link and pages with no edit link should not be a common use case, I think I'll use another approach directly in our fork until maybe one day a solution or another design is found.

What I will do is basically on desktop always have the edit link on the start, the last update date on the end, no matter if one of them is not displayed, and when switching to the layout with no sidebar (which is a fairly large breakpoint), I'll explicitly switch to have them both on 2 lines aligned to the start.

If some people are interested in this approach, here is a rough idea of the code I'll use:

diff --git a/packages/starlight/components/EditLink.astro b/packages/starlight/components/EditLink.astro
index 7ed74f3..2c2ccee 100644
--- a/packages/starlight/components/EditLink.astro
+++ b/packages/starlight/components/EditLink.astro
@@ -39,6 +39,7 @@ const url =
        align-items: center;
        text-decoration: none;
        color: var(--sl-color-gray-3);
+       flex: 1 0;
    }
    a:hover {
        color: var(--sl-color-white);
diff --git a/packages/starlight/components/Footer.astro b/packages/starlight/components/Footer.astro
index 1fae22f..df08878 100644
--- a/packages/starlight/components/Footer.astro
+++ b/packages/starlight/components/Footer.astro
@@ -42,10 +42,14 @@ const prevNextLinks = getPrevNextLinks(sidebar, config.pagination, {
 <style>
    .meta {
        gap: 0.75rem 3rem;
-       justify-content: space-between;
-       flex-wrap: wrap;
        margin-block: 3rem 1.5rem;
+       flex-direction: column;
        font-size: var(--sl-text-sm);
        color: var(--sl-color-gray-3);
    }
+   @media (min-width: 50rem) {
+       .meta {
+           flex-direction: row;
+       }
+   }
 </style>
diff --git a/packages/starlight/components/LastUpdated.astro b/packages/starlight/components/LastUpdated.astro
index 534da82..a6d73ef 100644
--- a/packages/starlight/components/LastUpdated.astro
+++ b/packages/starlight/components/LastUpdated.astro
@@ -35,3 +35,14 @@ try {
        </p>
    )
 }
+
+<style>
+   p {
+       flex: 1 0;
+   }
+   @media (min-width: 50rem) {
+       p {
+           text-align: end;
+       }
+   }
+</style>
lorenzolewis commented 1 year ago

I was able to get it with grid, but similar to @HiDeoo mentioned before I'm not a fan of that extra <div> needed, but that's what "pushes" the edit date all the way over to the side.

diff --git a/packages/starlight/components/Footer.astro b/packages/starlight/components/Footer.astro
index 1fae22f..6956d1a 100644
--- a/packages/starlight/components/Footer.astro
+++ b/packages/starlight/components/Footer.astro
@@ -21,8 +21,10 @@ const prevNextLinks = getPrevNextLinks(sidebar, config.pagination, {
 ---

 <footer>
-   <div class="meta flex">
+   <div class="meta">
+       <div>
        {config.editLink.baseUrl && <EditLink data={entry.data} id={entry.id} {locale} />}
+       </div>
        {
            (entry.data.lastUpdated ?? config.lastUpdated) && (
                <LastUpdated
@@ -41,11 +43,17 @@ const prevNextLinks = getPrevNextLinks(sidebar, config.pagination, {

 <style>
    .meta {
+       display: grid;
+       grid-template-columns: auto auto;
        gap: 0.75rem 3rem;
        justify-content: space-between;
-       flex-wrap: wrap;
        margin-block: 3rem 1.5rem;
        font-size: var(--sl-text-sm);
        color: var(--sl-color-gray-3);
    }
+       @media (max-width: 50rem) {
+           .meta {
+               grid-template-columns: auto;
+           }
+       }
 </style>
HiDeoo commented 1 year ago

Ah yeah, another workaround indeed, altho sadly won't fit the requirements from above to get it in Starlight too as it also relies on a breakpoint compared to the actual solution 😿 Is this one of theses cases that are truly impossible to solve? 🤣

lorenzolewis commented 1 year ago

Ah, so were you wanting the "breakpoint" to be if the row combined is longer than would fit on a single line? Not necessarily to change on the view-width breakpoint.

HiDeoo commented 1 year ago

That's how I understood this part from Chris message:

We rely on flex-wrap so that these items naturally wrap to two lines when they don’t fit (because we don’t reliably know their content: different dates, different languages, etc.), so can’t just throw a breakpoint at it.

It's crazy how the image represents something that looks relatively simple at first sight but how annoying it can be to actually get that ^^ Gotta love CSS.

lorenzolewis commented 1 year ago

Got another stab for ya:

diff --git a/packages/starlight/components/Footer.astro b/packages/starlight/components/Footer.astro
index 1fae22f..49d1e45 100644
--- a/packages/starlight/components/Footer.astro
+++ b/packages/starlight/components/Footer.astro
@@ -23,6 +23,7 @@ const prevNextLinks = getPrevNextLinks(sidebar, config.pagination, {
 <footer>
    <div class="meta flex">
        {config.editLink.baseUrl && <EditLink data={entry.data} id={entry.id} {locale} />}
+       <div></div>
        {
            (entry.data.lastUpdated ?? config.lastUpdated) && (
                <LastUpdated
@@ -48,4 +49,8 @@ const prevNextLinks = getPrevNextLinks(sidebar, config.pagination, {
        font-size: var(--sl-text-sm);
        color: var(--sl-color-gray-3);
    }
+
+   .meta > * {
+       white-space: nowrap;
+   }
 </style>

That extra div in the middle acts as a "spacer" (sort of inspired by SwiftUI). Again, requires adding another element to the DOM which is meh, but it does the trick.

HiDeoo commented 1 year ago

Ah, interesting approach, I think I might steal this one from you ^^ And it's one less file to patch compared to mine so even better.

It's a little be glitchy on some small sizes if a locale ever has very long strings in the footer but that's not the case with the locales we're dealing with so that'll be fine.

Thanks for sharing, really appreciated 🙌

lorenzolewis commented 1 year ago

I can put in a PR for it if you like @delucis, I can try to get it to wrap text if there's only 1 child in the row, but if that doesn't work I can do some ellipses instead.

delucis commented 1 year ago

Wow, appreciate the experimentation here. It’s like a CSS sudoku or something 😀

The spacer div have me an idea. Isn’t this what we want?

image

https://codepen.io/delucis/pen/QWJYOwx

Edit: Dang it… not quite… The “Last updated” is also pushed to the right on wide enough screens:

image

But maybe that’s OK?

HiDeoo commented 1 year ago

It’s like a CSS sudoku or something 😀

😆

Edit: Dang it… not quite… The “Last updated” is also pushed to the right on wide enough screens:

Yeah, I think I hit this behavior with one of the solutions and couldn't find a proper way to prevent it sadly 😢

But maybe that’s OK?

Personally, I think it is but that's only me ^^ What's your and everyone else's opinion on this?

delucis commented 1 year ago

Personally, I think it is but that's only me ^^ What's your and everyone else's opinion on this?

I think it’s OK too! Also, now realising this is the behaviour we’re happy with, the spacer is actually no longer relevant: https://codepen.io/delucis/pen/RwqvjjZ

Some equivalent of this targeting the updated date when it’s on its own is enough:

.meta > p:only-child {
  margin-inline-start: auto;
}
HiDeoo commented 1 year ago

Some equivalent of this targeting the updated date when it’s on its own is enough:

Oh yeah, you're perfectly right, even better 🥳