Skip to content

Better version listing for project dashboard #3673

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

Better version listing for project dashboard #3673

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6ba8cab
Better version listing for project dashboard - Issue: #3554
shubham76 Feb 24, 2018
3d2747c
restored projects/views/public.py
shubham76 Feb 24, 2018
15f3edd
Removed the extra comment
shubham76 Feb 24, 2018
7d0e431
made changes to check the right user
shubham76 Feb 25, 2018
4877aa3
Removed default branch name from the right side-bar
shubham76 Feb 27, 2018
bd67955
Reverted the project detail table styling
shubham76 Feb 28, 2018
d3d00d0
Update project_details.html
shubham76 Feb 28, 2018
28b9af2
removed extra spaces
shubham76 Mar 4, 2018
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
17 changes: 9 additions & 8 deletions readthedocs/templates/core/project_detail_right.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ <h3>{% trans "Project Privacy Level" %}</h3>
</div>
{% endblock %}

{% block project-default-branch %}
<h3>{% trans "Project Default Branch" %}</h3>
Copy link
Member

Choose a reason for hiding this comment

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

From #3554

The front dashboard page version listing should note the default branch

I think this info should go on the list, not on the side bar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh Yes. I thought so. But I wasn't clear about the requirement.
It is there on the list as shown in the picture.
Should I remove these lines then?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 on default version noted in the table, not the sidebar.

<div id="project-default-branch">
<p>
{{ project.get_default_branch }}
</p>
</div>
{% endblock %}

{% block short-urls %}
<h3>{% trans "Short URLs" %}</h3>
<p>
Expand Down Expand Up @@ -122,11 +131,3 @@ <h3>{% trans "Sub Projects" %}</h3>
</ul>
{% endif %}
{% endblock %}

{% block defaults %}
<h3>{% trans "Default Version" %}</h3>
<p> {{ project.get_default_version }}</p>

<h3>{% trans "'latest' Version" %}</h3>
<p> {{ project.get_default_branch }}</p>
{% endblock %}
62 changes: 32 additions & 30 deletions readthedocs/templates/core/project_details.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,40 +19,42 @@
<h3>{% trans "Versions" %}</h3>
</div>

<div class="module-list">
Copy link
Member

Choose a reason for hiding this comment

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

I think this div is still needed, can you post a screenshot of how it looks?

Copy link
Contributor Author

@shubham76 shubham76 Feb 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

tests-demo _ read the docs
Because of removing that div it looks like the version page listing.
What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, rtd is moving to match the previous style on all lists (see #3445 (comment))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But the issue #3554 here says it should match the style on project version listing page.
What should be done?

Copy link
Member

@stsewd stsewd Feb 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure, lets wait to agjohnson's reply, but on other issue this was already discussed (see #3572, relevant comments: #3572 (comment), #3572 (comment))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I saw that. I still went ahead with this issue as there were other improvements as well.
I hope that's fine. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

The styling that was removed here was indeed the correct styling, it should be added back. The styling wasn't the issue, but the table content. We're also missing an opportunity to have a single template for 3 different pages, so that is the larger focus here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! In that case, the previous design should remain same. Got it! I'll change the styling back again and keep the table content modified as it is in PR.

<div class="module-list-wrapper">
<ul>
{% for version in versions|sort_version_aware %}
<li class="module-item col-span">
{% if version.uploaded or version.built %}
{# Link to the docs #}
<a class="module-item-title" href="{{ version.get_absolute_url }}">
{{ version.slug }}
{% if request.user in project.users.all %}
Copy link
Member

Choose a reason for hiding this comment

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

You can use {% if request.user|is_admin:project %} for checking for the right user. You will need to load this tag {% load privacy_tags %} (if itsn't already loaded)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Right. I'll make the changes straight away.

<span class="right-menu quiet">{{ version.get_privacy_level_display }}</span>
{% endif %}

<div class="module-list-wrapper">
<ul>
{% for version in versions|sort_version_aware %}
<li class="module-item col-span">
{% if version.uploaded or version.built %}
{# Link to the docs #}
<a class="module-item-title" href="{{ version.get_absolute_url }}">
{{ version.slug }}
{% if request.user in project.users.all %}
<span class="right-menu quiet">{{ version.get_privacy_level_display }}</span>
{% endif %}
</a>
{% else %}
<a class="module-item-title" href="{{ version.project.get_builds_url }}">
{{ version.slug }}
</a>
{% if not version.slug in version.identifier %}
<span class="right-menu quiet version-branch-privacy">{{ version.identifier_friendly|truncatechars:24 }}</span>
{% endif %}
</a>
{% else %}
<a class="module-item-title" href="{{ version.project.get_builds_url }}">
{{ version.slug }}
</a>
{% endif %}
{% if request.user|is_admin:project %}
<ul class="module-item-menu">
<li><a href="{% url "project_version_detail" project.slug version.slug %}">{% trans "Edit" %}</a></li>
</ul>
{% endif %}
{% if request.user|is_admin:project %}
<ul class="module-item-menu">
<li><a href="{% url "project_version_detail" project.slug version.slug %}">{% trans "Edit" %}</a></li>
</ul>
{% endif %}
</li>
</li>

{% empty %}
<p>
{% trans "No active versions." %}
</p>
{% endfor %}
</ul>
</div>
{% empty %}
<p>
{% trans "No active versions." %}
</p>
{% endfor %}
</ul>
</div>


{% block build_versions %}
{% if versions and request.user|is_admin:project %}
Expand Down