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 7 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
8 changes: 0 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 @@ -122,11 +122,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 %}
11 changes: 8 additions & 3 deletions readthedocs/templates/core/project_details.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
<div class="module-header">
<h3>{% trans "Versions" %}</h3>
</div>

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove Extra spaces !

Copy link
Contributor Author

@shubham76 shubham76 Mar 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Well, it has been like that in the original code. https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/templates/core/project_details.html. I did not try to change that style.

Copy link
Contributor

@bansalnitish bansalnitish Mar 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The line is fine. I am referring to the extra whitespace you have added. It was not in the original code.

<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.


Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto !

<div class="module-list-wrapper">
<ul>
{% for version in versions|sort_version_aware %}
Expand All @@ -29,9 +29,13 @@ <h3>{% trans "Versions" %}</h3>
{# Link to the docs #}
<a class="module-item-title" href="{{ version.get_absolute_url }}">
{{ version.slug }}
{% if request.user in project.users.all %}
{% if request.user|is_admin:project %}
<span class="right-menu quiet">{{ version.get_privacy_level_display }}</span>
{% endif %}

{% 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 }}">
Expand All @@ -53,6 +57,7 @@ <h3>{% trans "Versions" %}</h3>
</ul>
</div>
</div>


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