Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add guideline to last step of the <Steps> component #2051

Merged
merged 6 commits into from
Aug 16, 2024

Conversation

HiDeoo
Copy link
Member

@HiDeoo HiDeoo commented Jun 25, 2024

Description

The initial design of the <Steps> component does not include a guideline for the last step:

current

Based on some feedback, this PR introduces a guideline for the last step to help visualizing the context of the last step, specially for long last steps.

Different approaches are possible:

Using the existing guideline Fading the guideline linearly Fading the guideline in the last 5rem
plain gradient-50 gradient-end

This PR uses the third approach, fading the guideline near the end of the last step.

I'm totally open to any feedback regarding the approach or values used.

Remaining tasks

  • Remove the docs/src/content/docs/guides/steps.mdx file

Copy link

changeset-bot bot commented Jun 25, 2024

🦋 Changeset detected

Latest commit: 6a39d38

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/starlight Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

This comment was marked as outdated.

@github-actions github-actions bot added 📚 docs Documentation website changes 🌟 core Changes to Starlight’s main package labels Jun 25, 2024
@astrobot-houston
Copy link
Collaborator

astrobot-houston commented Jun 25, 2024

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

Locale File Note
en guides/steps.mdx Source added, will be tracked.
Warnings reference
Icon Description
🔄️ The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

@lorenzolewis
Copy link
Contributor

Really like this! The only thing I'm not a fan of are the one-line last steps. I would prefer no trailing line if only a single line, but I can understand that may be tricky (if even possible at all) to do with text wrapping (unless using some magic-numbers).

@evadecker
Copy link
Contributor

Fade-outs are often used to imply that content continues off-screen or is truncated. When I see the guideline fade out it makes me think that there are additional steps hidden from view.

I think the existing design is preferable, even for long final steps. The content is indented, which communicates its hierarchy and relation to the number. No extra visuals needed.

@HiDeoo
Copy link
Member Author

HiDeoo commented Jul 10, 2024

The only thing I'm not a fan of are the one-line last steps.

This is indeed something I noticed as well but did not bother me. I'll try to see if there is some kind of trick we can use but I cannot think of an obvious one right now.

The content is indented, which communicates its hierarchy and relation to the number. No extra visuals needed.

I would guess this highly depends on the content and your screen size. If you're in the middle of a relatively long step, or on a phone or small window and cannot see the previous/next step number indicator, you have no point of reference to know the content you're seeing is indented. And this seems to be the case for most of the negative feedback I got on this. Definitely not saying this solution is the proper one ofc (it's just the only one we thought of so far), but I think there may be some room for improvement on our side to address the issue.

Copy link

netlify bot commented Jul 22, 2024

Deploy Preview for astro-starlight ready!

Name Link
🔨 Latest commit 6a39d38
🔍 Latest deploy log https://app.netlify.com/sites/astro-starlight/deploys/66bf1c15faea200008cd9d1d
😎 Deploy Preview https://deploy-preview-2051--astro-starlight.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks again for kicking this off and sharing the feedback you received @HiDeoo! And thanks for chipping in everyone else.

Now that I’m looking at this in context, I also have the feeling that the fade-out immediately makes me think of truncated or hidden content — kind of like when you see content fade out and then a subscribe banner appears or a “see more” button.

I wonder if actually the simpler solid line is the better choice given this?

Regarding only adding the line when there’s enough content. This is indeed tricky. Something like the following does work in browsers that support :has():

/* Hide guide for last list item that doesn’t have a second child (i.e. has just one child) */
.sl-steps > li:last-of-type:not(:has(> *:nth-child(2)))::after {
  background: transparent;
}

But then there’s the question of whether the inconsistency is not worth this optimization. For example:

Two sets of steps. In the first, the final step show no guideline because it contains just one paragraph, wrapped onto three lines. In the second, the final step shows a guideline because it contains two paragraphs, which do not wrap.

The final CSS trick that could be possible is a container query for the height of the step? Although I’m not sure li::after could query the container if the container is defined by li. In that case it could even be something pretty context dependent like “if the container is taller than the 90vh, show the guide line” theoretically. Ignore this bit, auto-sized containers on the block axis is not possible 😁

@HiDeoo
Copy link
Member Author

HiDeoo commented Jul 23, 2024

I wonder if actually the simpler solid line is the better choice given this?

I think this is perfectly fine too (it was one of the 3 suggested approaches in the original post) and it also addresses the issue. I updated the PR to remove the gradient.

As we discussed, the one-line case is indeed tricky and inconsistent and I don't think there is even an upcoming CSS feature that would allow us to address this in a more elegant/reliable way.

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks again @HiDeoo! I think this is a reasonable compromise. It may not be the most beautiful, but it does at least match how other <Steps> components I’ve found work and I think the user feedback we had is worth listening to.

Left a couple of notes around how we release this, but otherwise this change LGTM! (I guess it is only a single-line change after all 😁)

.changeset/swift-laws-yawn.md Outdated Show resolved Hide resolved
.changeset/swift-laws-yawn.md Show resolved Hide resolved
docs/src/content/docs/guides/steps.mdx Outdated Show resolved Hide resolved
HiDeoo and others added 2 commits July 24, 2024 09:07
Co-authored-by: Chris Swithinbank <357379+delucis@users.noreply.github.com>
@github-actions github-actions bot removed the 📚 docs Documentation website changes label Jul 24, 2024
@delucis delucis added the 🌟 minor Change that triggers a minor release label Jul 24, 2024
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks again for working through this @HiDeoo! Should be good to go for our next minor 🫡

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 core Changes to Starlight’s main package 🌟 minor Change that triggers a minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants