Build popups: simplification #414
Conversation
This commit simplifies the build chip a little: - removes the `text` for the list item since it always repeat the name of the project and there is no need to show it so many times - removes the name of the project from the popup title for the same reason - adds the text "Build" to the title since it conveys a more clear meaning - makes the build number clickable, so we can go to the build details page from the chip's title
| @@ -30,7 +32,7 @@ | |||
| {% comment %} | |||
| This block is the same as the chip_text block above. | |||
| {% endcomment %} | |||
| {% firstof text build.get_version_name %} #{{ build.id }} | |||
| Build <a href="{% url 'builds_detail' build.project.slug build.pk %}">#{{ build.id }}</a> | |||
There was a problem hiding this comment.
This should be marked as translatable.
There was a problem hiding this comment.
Also, none of the other cards have linkable titles, as the chip is linked already.
For now, the full title should be linked, not just a partial title. This makes translation easier.
For later though:
- Link does not display as blue text, this doesn't look as nice
- We should do this at the base template so that all popupcards operate the same way.
| @@ -183,7 +183,7 @@ | |||
| {% with build=object.last_build %} | |||
| {% if build %} | |||
| <div class="item"> | |||
| {% include "includes/elements/chips/build.html" with build=build text=object.project.name %} | |||
| {% include "includes/elements/chips/build.html" with build=build notext=True %} | |||
There was a problem hiding this comment.
Another parameter isn't required here, we can use text=" " to still satisfy firstof.
However, I'll note below why I think we don't want this.
| {% firstof text build.get_version_name %} | ||
| {% if not notext %} | ||
| {% firstof text build.get_version_name %} | ||
| {% endif %} |
There was a problem hiding this comment.
This does need some text, for a couple reasons:
- This is the main content of the
.ui.labeland it looks too padded/empty without this - The other popup cards use all this same pattern - icon, main content, and detail content
While I agree repeating the project name isn't helpful, it is consistent. For comparison, CircleCI also is redundant when describing builds:
This states the project name as the word Build looks even more redundant and isn't useful information in addition:
So, I think project name is best, maybe Build is okay, but this text should be something.
We should revisit patterns here later. I have a pile of work on popup cards in the milestone https://github.com/readthedocs/ext-theme/milestone/6
This commit simplifies the build chip a little:
textfor the list item since it always repeat the name of the project and there is no need to show it so many times