Skip to content

Make doc preview more prominent and remove headings #12295

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ericholscher
Copy link
Member

@ericholscher ericholscher commented Jul 3, 2025

I think the content is self-explanatory,
and the huge header is not adding value.
I think the primary action should be the biggest text.
I'd like to see that become a button,
but baby steps.

I think the content is self-explanatory,
and the huge header is not adding value.
I think the primary action should be the biggest text.
I'd like to see that become a button though.
@ericholscher ericholscher requested a review from a team as a code owner July 3, 2025 09:35
@ericholscher ericholscher requested a review from stsewd July 3, 2025 09:35
Copy link

Documentation build overview

📚 dev | 🛠️ build #28730295 (c1cb4ee) | 🔍 preview

Files changed

Comparing with latest (4617dfa)

No files changed.

Copy link

Documentation build overview

📚 docs | 🛠️ build #28730296 (c1cb4ee) | 🔍 preview

Files changed

Comparing with latest (4617dfa)

Show files (4) | 3 modified | 1 added | 0 deleted
File Status
privacy-policy.html 📝 modified
terms-of-service.html ➕ added
unofficial-projects.html 📝 modified
reference/policies.html 📝 modified

@humitos
Copy link
Member

humitos commented Jul 3, 2025

I'd like to see that become a button,

We can probably use a badge here or a quick solution is to use kbd. Examples https://github.com/MarkedDown/Buttons

@humitos
Copy link
Member

humitos commented Jul 3, 2025

I think we all have different taste here for how this comment should be and it seems we will be iterating quickly in the next weeks/months, so I'd suggest to mark this feature as beta and communicate clearly what to expect at this early stage.

@ericholscher
Copy link
Member Author

I think it's very verbose now, so we should definitely get three changes or similar in.

@ericholscher
Copy link
Member Author

@stsewd I'd still like to see something like this make it in. I find the current message has a lot of text that isn't really needed.

### Files changed

> Comparing with [{{ base_version.verbose_name }}]({{ base_version.get_absolute_url }}) ({{ base_version_build.commit }}...{{ current_version_build.commit }})
> 📚 [{{ project.name }}](https://{{ PRODUCTION_DOMAIN }}{% url "projects_detail" project.slug %}) | 🛠️ Build [#{{ current_version_build.pk }}](https://{{ PRODUCTION_DOMAIN }}{% url "builds_detail" project.slug current_version_build.pk %}) ({{ current_version_build.commit }}) | comparing [{{ base_version.verbose_name }}]({{ base_version.get_absolute_url }}) ({{ base_version_build.commit }}...{{ current_version_build.commit }})
Copy link
Member

Choose a reason for hiding this comment

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

everything is lowercase here

Suggested change
> 📚 [{{ project.name }}](https://{{ PRODUCTION_DOMAIN }}{% url "projects_detail" project.slug %}) | 🛠️ Build [#{{ current_version_build.pk }}](https://{{ PRODUCTION_DOMAIN }}{% url "builds_detail" project.slug current_version_build.pk %}) ({{ current_version_build.commit }}) | comparing [{{ base_version.verbose_name }}]({{ base_version.get_absolute_url }}) ({{ base_version_build.commit }}...{{ current_version_build.commit }})
> 📚 [{{ project.name }}](https://{{ PRODUCTION_DOMAIN }}{% url "projects_detail" project.slug %}) | 🛠️ build [#{{ current_version_build.pk }}](https://{{ PRODUCTION_DOMAIN }}{% url "builds_detail" project.slug current_version_build.pk %}) ({{ current_version_build.commit }}) | comparing [{{ base_version.verbose_name }}]({{ base_version.get_absolute_url }}) ({{ base_version_build.commit }}...{{ current_version_build.commit }})


> 📚 [{{ project.name }}](https://{{ PRODUCTION_DOMAIN }}{% url "projects_detail" project.slug %}) | 🛠️ build [#{{ current_version_build.pk }}](https://{{ PRODUCTION_DOMAIN }}{% url "builds_detail" project.slug current_version_build.pk %}) ({{ current_version_build.commit }}) | 🔍 [preview]({{ current_version.get_absolute_url }})
#### [View documentation preview]({{ current_version.get_absolute_url }})
Copy link
Member

Choose a reason for hiding this comment

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

Not really a fan of having this link in the header TBH.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I don't like it either.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feel the heading is wrong. If you look at other providers, like Sentry, their comments have the same structure and include a heading.

Copy link
Member

Choose a reason for hiding this comment

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

The heading is not bad, what I don't like is the heading being a link

@humitos
Copy link
Member

humitos commented Jul 29, 2025

Documentation build overview (preview)

Files changed

Comparing with latest (4617dfa)

No files changed.

@humitos
Copy link
Member

humitos commented Jul 29, 2025

The previous comment is my proposal for this message.

@ericholscher
Copy link
Member Author

ericholscher commented Jul 29, 2025

There's still a ton of headings that take up space for no reason. We don't need a Files changed heading, we should just list the files that changes. The headings take a ton of space for no value. I'm OK with the primary heading I guess, but I still think it's too big and too spaced out. I think we want minimal as much as possible.

Your suggestion has Preview twice, and files changed twice. It's way too verbose.

@humitos
Copy link
Member

humitos commented Jul 29, 2025

Documentation build overview

Comparing with latest (4617dfa):

Show files (2) | 1 modified | 1 added | 0 deleted
File Status
privacy-policy.html 📝 modified
terms-of-service.html ➕ added

@humitos
Copy link
Member

humitos commented Jul 29, 2025

I put another proposal based on the feedback.

@humitos
Copy link
Member

humitos commented Jul 29, 2025

Documentation build overview


 📚 Project (dev) 

 🛠️ Build 

 🔍 Preview 

Comparing with latest (4617dfa):

Show files (2) | 1 modified | 1 added | 0 deleted
File Status
privacy-policy.html 📝 modified
terms-of-service.html ➕ added

@ericholscher
Copy link
Member Author

Something like that looks good, but we should definitely have viewing the preview as the primary action. I think that's the biggest thing people are going to want, and it should be the most visible. All the other items are more metadata, and less likely to be useful.

@agjohnson
Copy link
Contributor

I have mostly the same feedback there. I think we can keep the first header, the list of changes makes plenty of sense without a "Files changed" header though.

But we can both retain the header and reduce the metadata sprawl by adding useful metadata into the heading instead. This is okay because users really shouldn't need to click into this metadata anyways. Users won't have a reason to click into the project from the comment and this comment will only reflect passing builds, so the build link is mostly useless to the user too -- users don't need to review passing build details.

I'd still keep the preview link action out of the header entirely, the button is most clear. Using a button for the other metadata makes the function unclear though -- ie, does the "Build" button trigger a new build?

The vertical list of metadata is mostly more wordy than the horizontal. We nixed this last review, I think the horizontal list still looks better. But I think we can get rid of this line entirely.

Altogether, below:

  • Combine metadata and header, put not super useful metadata links in the header
  • Drops the metadata line
  • Keep useful metadata like commit/version as a separate line
  • Clarify context around changes list and emoji usage, remove redundant copy from changes table
  • Use kbd element for the preview button -- note the added spacing fix
  • Put the action last, like you would normally encounter in a UI

This feels dense and none of the copy/data seems like filler content.


Overview for project dev build #28730295

Comparing c1cb4ee with latest (4617dfa):

Show files changed (2 files total: 📝 1 modified | ➕ 1 added | ➖ 0 deleted)
File Status
privacy-policy.html 📝
terms-of-service.html


 🔍 Preview build  

@humitos
Copy link
Member

humitos commented Jul 30, 2025

I don't like the links on the header. They look too big and prominent to me, generating the opposite effect we want here (it shouldn't be the main action to do). Also, dev is a very small project slug, but this will look terrible for projects with a few more characters in the slug, event worse for projects in commercial, where we prefix the organization's slug.

I propose to put these links outside the header. I prefer the bullets for this, but if we don't like them, I'm fine going back to the one line we had before.

Based on these feedback, I have another proposal that's just a small header, 2 lines and the "Preview build" button. Looks pretty compact to me.


Documentation build overview

📚 Project dev | 🛠️ Build #28730295 (c1cb4ee) | 📁 Comparing c1cb4ee with latest (4617dfa)

Show files changed (2 files total: 📝 1 modified | ➕ 1 added | ➖ 0 deleted)
File Status
privacy-policy.html 📝
terms-of-service.html


 🔍 Preview build  

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants