Skip to content

Display warning banner in latest version #3547

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 3 commits into from
Closed

Display warning banner in latest version #3547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f05090f
Return is_stable from API
stsewd Jan 24, 2018
670078c
Show appropriate message on latest
stsewd Jan 24, 2018
642fa54
Remove log messages
stsewd Jan 24, 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
11 changes: 8 additions & 3 deletions readthedocs/core/static-src/core/js/doc-embed/version-compare.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,22 @@ function init(data) {

/// Out of date message

if (data.is_highest) {
if (data.is_stable) {
return;
}

var message = 'You are not using the most up to date version of the library.';
if (data.is_highest) {
message = 'You are not using the stable version of the library.';
Copy link
Member

Choose a reason for hiding this comment

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

Somewhere I read that @ericholscher used the word "unreleased" here instead and I liked.

I think it's clearer to use a sentence like,

You are using an unreleased version of this library

Going deeper on this (maybe in another PR focusing more on this), words like "library" and "using" don't make too much sense for all type of documentation that RTD hosts. Maybe changing them to "documentation" and "reading" are more general.

You are reading an unreleased version of this documentation

What do you think?

Example from Django docs,

This document is for Django's development version, which can be significantly different from previous releases. For older releases, use the version selector floating in the bottom right corner of this page.

https://docs.djangoproject.com/en/dev/intro/tutorial01/

I also like the "tip" saying where you can find the other versions. I'd like to go in that direction, but I'm not sure how to write a good English sentence for this, @RichardLitt want to help us?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are reading an unreleased version of this documentation

I was thinking the same, the word documentation makes more sense here.

Also, we are missing some i18n on this message, I'm not sure how relevant is that for now.

About the tip, I think would be a little confusing, since the position of the version selector change according to the theme (some times is on the left and others floating to the right), and we asre already providing a link to go to the stable version.

Copy link
Member

Choose a reason for hiding this comment

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

Also, we are missing some i18n on this message, I'm not sure how relevant is that for now.

Good point! I think it's relevant, but I'd say to open another issue to track this. I don't know how we are handling i18n in Javascript. Did you see other parts in the code?

About the tip, I think would be a little confusing [...]

Yeah, you are right. I think there is no generic way to point the users to the right place.

}

var currentURL = window.location.pathname.replace(rtd['version'], data.slug);
var warning = $(
'<div class="admonition warning"> ' +
'<p class="first admonition-title">Note</p> ' +
'<p class="last"> ' +
'You are not using the most up to date version of the library. ' +
'<a href="#"></a> is the newest version.' +
message + ' ' +
'<a href="#"></a> is the stable version.' +
'</p>' +
'</div>');

Expand Down
10 changes: 8 additions & 2 deletions readthedocs/restapi/views/footer_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from rest_framework.response import Response
from rest_framework_jsonp.renderers import JSONPRenderer

from readthedocs.builds.constants import LATEST, TAG
from readthedocs.builds.constants import LATEST, STABLE, TAG
from readthedocs.builds.models import Version
from readthedocs.projects.models import Project
from readthedocs.projects.version_handling import (
Expand All @@ -26,7 +26,8 @@ def get_version_compare_data(project, base_version=None):
Retrieve metadata about the highest version available for this project.

:param base_version: We assert whether or not the base_version is also the
highest version in the resulting "is_highest" value.
highest version in the resulting "is_highest" value
or the stabel version in "is_stable".
Copy link
Member Author

Choose a reason for hiding this comment

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

Little typo "stabel" -> "stable"

"""
versions_qs = project.versions.public().filter(active=True)

Expand All @@ -40,10 +41,15 @@ def get_version_compare_data(project, base_version=None):
'project': six.text_type(highest_version_obj),
'version': six.text_type(highest_version_comparable),
'is_highest': True,
'is_stable': False,
}
if highest_version_obj:
ret_val['url'] = highest_version_obj.get_absolute_url()
ret_val['slug'] = (highest_version_obj.slug,)
if base_version and base_version.slug == STABLE:
ret_val['is_highest'] = False
ret_val['is_stable'] = True
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really following the whole flow, but supposing this case:

  • base_version is the STABLE
  • we get a highest_version_obj

we end up returning a is_stable in True but with the url and slug from another version?

(I'm asking because I'm a little confused here)

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I'm confused too p: but looking to the docstring, says:

Retrieve metadata about the highest version available for this project.

So, always return that information about the highest. And when base_version is given, is_stable and is_highest are for that version. I think that method needs refactoring.

And I realized that when I enter to stable version manually (docs/2018 instead of docs/stable in my example) the message appears, I need to check that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

After looking deeper into the code, I found that highest == stable, or at least the function return the mayor version (except for latest, stable) of the project.

@humitos can you confirm that? In that case, we need to add some field is_latest, no is_stable, or keep the names of the current PR and change the logic (I'm not sure if this breaks other part of the code that uses the API).

return ret_val
if base_version and base_version.slug != LATEST:
try:
base_version_comparable = parse_version_failsafe(
Expand Down