Skip to content

Update build_name to make it a bit more explicit #615

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 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 34 additions & 38 deletions readthedocsext/theme/templates/builds/includes/build_name.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,51 +33,47 @@

{% endcomment %}

{% load i18n %}
{% from i18n load trans blocktrans %}

{# When not a page title, treat the entire breadcrumb as a link to the build #}
{% if not is_page_title %}<a href="{{ build.get_absolute_url }}">{% endif %}
<span class="ui {% if is_page_title %}large{% endif %} breadcrumb">
<span class="section">
{# When used a a page title, link the sections independently #}
{% if is_page_title %}
{% if is_page_title %}
<span class="ui large breadcrumb">
<span class="section">
<a href="{% url "projects_detail" build.project.slug %}?slug={{ build.get_version_slug }}">
{% endif %}

{% if build.is_external %}
{{ build.external_version_name | lower | capfirst }}
{{ build.get_version_name }}
{% else %}
{# Translators: this renders with the version name, example "Version latest" #}
{% blocktrans trimmed with version_name=build.get_version_name %}
Version {{ version_name }}
{% endblocktrans %}
{% endif %}

{% if is_page_title %}
{% if build.is_external %}
{{ build.external_version_name | lower | capfirst }}
{{ build.get_version_name }}
{% else %}
{# Translators: this renders with the version name, example "Version latest" #}
{% blocktrans trimmed with version_name=build.get_version_name %}
Version {{ version_name }}
{% endblocktrans %}
{% endif %}
</a>
{% endif %}
</span>
{% if is_page_title %}
{# When used as a page title, show a section link for the filtered build list #}
</span>
<span class="divider">/</span>
{# Translators: this refers to a list of builds for a single project #}
<a class="section" href="{% url "builds_project_list" build.project.slug %}?version__slug={{ build.get_version_slug }}">
<a class="section"
href="{% url "builds_project_list" build.project.slug %}?version__slug={{ build.get_version_slug }}">
{% trans "Builds" %}
</a>
{% endif %}
<span class="divider">/</span>
<span class="active section">
{% if is_page_title %}
<span class="divider">/</span>
<span class="active section">
<a href="{{ build.get_absolute_url }}">
{% endif %}
<span class="ui grey text">
#{{ build.pk }}
</span>
{% if is_page_title %}
<span class="ui grey text">#{{ build.pk }}</span>
</a>
{% endif %}
</span>
</span>
</span>
</a>
{% if not is_page_title %}</a>{% endif %}
{% else %}
<a href="{{ build.get_absolute_url }}">
<span class="ui breadcrumb">
<span class="active section">
<span class="ui grey text">
{# Translators: this shows the build number, example "Build #1234" #}
{% blocktrans trimmed with build_id=build.pk %}
Build #{{ build_id }}
{% endblocktrans %}
</span>
</span>
</span>
</a>
Comment on lines +67 to +78
Copy link
Contributor

Choose a reason for hiding this comment

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

So yeah as you were saying it would be better to just skip the include. With this change you aren't showing any other breadcrumbs, so we don't want to use that element anymore. And we don't want to show the text as grey anymore either, in line with all of the other listing views. It was only grey to visually demote the id.

So this becomes:

Suggested change
<a href="{{ build.get_absolute_url }}">
<span class="ui breadcrumb">
<span class="active section">
<span class="ui grey text">
{# Translators: this shows the build number, example "Build #1234" #}
{% blocktrans trimmed with build_id=build.pk %}
Build #{{ build_id }}
{% endblocktrans %}
</span>
</span>
</span>
</a>
<a href="{{ build.get_absolute_url }}">
{# Translators: this shows the build number, example "Build #1234" #}
{% blocktrans trimmed with build_id=build.pk %}
Build #{{ build_id }}
{% endblocktrans %}
</a>

{% endif %}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
data-bind="semanticui: { popup: { content: '{{ object.date }}', position: 'top center', delay: { show: 500 }, variation: 'small'}}">
{# Translators: this will read like "Started 1 month, 3 days ago" #}
{% blocktrans with object.date|naturaltime as date trimmed %}
Started {{ date }}
{{ date }}
Copy link
Contributor

@agjohnson agjohnson Jun 19, 2025

Choose a reason for hiding this comment

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

Oh and not sure on the reason for removal here, but this feels too terse without prefix text. There should be some context around this date. "5 minutes ago" is ambigious -- started 5 minutes ago or finished 5 minutes ago?

{% endblocktrans %}
</div>
</div>
Expand Down