Display warning banner in latest version #3547

stsewd · 2018-01-24T20:04:50Z

Closes #3481 and probably solves #1644

I introduced a new field in the API response: is_stable, to indicate if the given version is stable.
Now the highest version is only for the latest version, stable isn't considered highest anymore.

On stable version

On latest version

On old version

Now the highest version is only for the latest version, stable isn't considered highest.

stsewd · 2018-01-24T20:20:52Z

I need to update the tests

stsewd · 2018-01-24T20:48:13Z

readthedocs/restapi/views/footer_views.py

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


Little typo "stabel" -> "stable"

humitos

Thanks for your contribution!

I think this is a good improvement.

It changes a little the old logic since RTD was sending all the people to read the most updated unreleased documentation and now RTD will be sending people to read the "latest stable" release. I personally think that it's a good improvement and it's the proper direction but I wanted to mention this in case that other people find this inapropiated for some reason (that I'd like to listen)

Finally, I left two comments to consider and discuss a little.

humitos · 2018-01-25T01:18:22Z

readthedocs/restapi/views/footer_views.py

    }
    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


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)

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.

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

humitos · 2018-01-25T01:35:58Z

readthedocs/core/static-src/core/js/doc-embed/version-compare.js

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


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?

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.

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.

humitos · 2018-04-02T18:25:31Z

I wrote a comment at #3481 (comment) that, if implemented, will cover this case in a general sense.

ericholscher · 2018-05-30T21:13:50Z

@stsewd thoughts here? Is this still something we should do? I'm not sure..

stsewd · 2018-05-30T21:16:36Z

I think with #3595 this was solved

Jackenmen · 2020-02-18T10:52:36Z

ReadTheDocs still doesn't display warning banner in latest version so the part this PR would change is still not solved.

stsewd · 2020-02-18T14:05:25Z

ReadTheDocs still doesn't display warning banner in latest version so the part this PR would change is still not solved.

No, we don't display the banner in the latest version, but on the "latest stable" version

Jackenmen · 2020-02-18T14:17:36Z

@stsewd I meant that currently this is readthedocs.org's behavior (assuming 1.0.1 is the "latest stable" version):

1.0.0 - warning about not using the most recent version of documentation
1.0.1 - no warning
stable - no warning
latest - no warning

While this PR would change it to something like this:

1.0.0 - warning about not using the most recent version of documentation
1.0.1 - no warning
stable - no warning
latest - warning about using an unreleased version of documentation

Having such warning in latest docs is in my opinion very good to have so that people don't use "unreleased" version of documentation without proper warning and get linked to stable one.

Edit: I did read this comment though so this would probably need to not be as simple as this PR and it should be possible to disable such behavior (and customize message shown?). This is just the most suitable place that I was able to find about the mentioned change of behavior.

stsewd · 2020-02-18T14:20:09Z

@jack1142 you can use this extension to have more control over which versions the banner is shown https://github.com/humitos/sphinx-version-warning

stsewd added 3 commits January 24, 2018 14:56

Return is_stable from API

f05090f

Now the highest version is only for the latest version, stable isn't considered highest.

Show appropriate message on latest

670078c

Remove log messages

642fa54

stsewd commented Jan 24, 2018

View reviewed changes

humitos reviewed Jan 25, 2018

View reviewed changes

stsewd mentioned this pull request Jan 28, 2018

Missing i18n for footer and warning banner #3561

Closed

lb- mentioned this pull request Feb 6, 2018

Warn people when they're looking at the development version of the docs wagtail/wagtail#3375

Closed

stsewd mentioned this pull request Feb 11, 2018

Allow to hide version warning #3595

Merged

stsewd added the Needed: design decision A core team decision is required label Apr 11, 2018

stsewd closed this May 30, 2018

stsewd deleted the display-banner-in-latest branch February 27, 2019 22:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display warning banner in latest version #3547

Display warning banner in latest version #3547

stsewd commented Jan 24, 2018 •

edited

Loading

stsewd commented Jan 24, 2018

stsewd Jan 24, 2018

humitos left a comment

humitos Jan 25, 2018

stsewd Jan 25, 2018

stsewd Jan 25, 2018

humitos Jan 25, 2018

stsewd Jan 25, 2018

humitos Jan 26, 2018

humitos commented Apr 2, 2018

ericholscher commented May 30, 2018

stsewd commented May 30, 2018

Jackenmen commented Feb 18, 2020

stsewd commented Feb 18, 2020

Jackenmen commented Feb 18, 2020 •

edited

Loading

stsewd commented Feb 18, 2020

Display warning banner in latest version #3547

Display warning banner in latest version #3547

Conversation

stsewd commented Jan 24, 2018 • edited Loading

On stable version

On latest version

On old version

stsewd commented Jan 24, 2018

stsewd Jan 24, 2018

Choose a reason for hiding this comment

humitos left a comment

Choose a reason for hiding this comment

humitos Jan 25, 2018

Choose a reason for hiding this comment

stsewd Jan 25, 2018

Choose a reason for hiding this comment

stsewd Jan 25, 2018

Choose a reason for hiding this comment

humitos Jan 25, 2018

Choose a reason for hiding this comment

stsewd Jan 25, 2018

Choose a reason for hiding this comment

humitos Jan 26, 2018

Choose a reason for hiding this comment

humitos commented Apr 2, 2018

ericholscher commented May 30, 2018

stsewd commented May 30, 2018

Jackenmen commented Feb 18, 2020

stsewd commented Feb 18, 2020

Jackenmen commented Feb 18, 2020 • edited Loading

stsewd commented Feb 18, 2020

stsewd commented Jan 24, 2018 •

edited

Loading

Jackenmen commented Feb 18, 2020 •

edited

Loading