Skip to content
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.

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

gui: Generate version-specific documentation link URLs #8108

Merged
merged 6 commits into from
Jan 24, 2022

Conversation

acolomb
Copy link
Member

@acolomb acolomb commented Jan 11, 2022

One puzzle piece to fix #8107. Should be merged only when the other pieces are deployed.

Replace all HTML references to https://docs.syncthing.net with a function call to return a version-specific URL. Use the currently running version string up to the first dash (-) character as a subdirectory name on the docs site.

Fall back to the generic base URL on error.

Replace all HTML references to https://docs.syncthing.net with a
function call to return a version-specific URL.  Use the currently
running version string up to the first dash (-) character as a
subdirectory name on the docs site.

Fall back to the generic base URL on error.
Remove docsBaseURL, use more compatible slice() instead of
substring().
@acolomb acolomb marked this pull request as ready for review January 20, 2022 16:44
gui/default/index.html Outdated Show resolved Hide resolved
Comment on lines +2902 to +2916
$scope.docsURL = function (path) {
var url = 'https://docs.syncthing.net';
if (path) {
var hash = path.indexOf('#');
if (hash != -1) {
url += '/' + path.slice(0, hash);
url += '?version=' + $scope.versionBase();
url += path.slice(hash);
} else {
url += '/' + path;
url += '?version=' + $scope.versionBase();
}
}
return url;
};
Copy link
Member

Choose a reason for hiding this comment

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

I realized something interesting about our docs serving at the moment, in that most of the old URLs here ended with .html but one didn't, and the one that didn't still worked because apparently Netlify does magic and adds it internally instead of 404:ing. That makes the above code (which omits the .html) work, and means we can lay the foundation for a switch to generating nicer docs urls with make dirhtml without breaking everything...

Copy link
Member

Choose a reason for hiding this comment

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

It is however irreversible in the sense that we'll then have versions out there referring to both /foo and /foo.html and we'll probably need to handle both variants server side for a long while or risk breaking some subset of users.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, since there have been many versions now with mixed variants, we're bound to keep the fall-back behavior on the server. Should be relatively easy to do though on any hosting? Just needs to be remembered. And in the end, having some docs links from ancient versions stop working someday isn't the end of the world. The internal links within the docs are correct anyway. So that's a go in leaving out .html from now on?

calmh
calmh previously approved these changes Jan 22, 2022
@tomasz1986
Copy link
Contributor

Is there any indication on the Docs page that we're actually browsing a version-specific site rather than the main one? If not, then I'd strongly suggest to either have the version number written somewhere visible on the screen or, if possible, to have a dropdown menu allowing the user to select between different versions.

Please correct me if I'm wrong, but at the moment it seems that it is only the URL that is clearly version-specific. If that is the case, then I'd say it's really not enough, because the vast majority of users will simply fail to notice it, and then they'll wonder what is going on (especially since up to now the GUI has used to simply link to the main Docs site). Also, on small screens, that part of the URL may very well be hidden unless the user clicks the address bar and scrolls it.

In other words, I think there are two potential problems. Firstly, the user will likely be unaware that they're browsing version-specific documentation. Secondly, there's no user-friendly way of switching to the current or between different Docs versions.

@calmh
Copy link
Member

calmh commented Jan 22, 2022

Why would they wonder what's going on? What scenario are you envisioning causing the confusion? I mean, I agree we should show a "You're browsing potentially outdated documentation" banner, and we'll get to that from the documentation side, but I don't see how it's critical here.

@tomasz1986
Copy link
Contributor

tomasz1986 commented Jan 22, 2022

Well, I, for one, am used to be able to quickly jump to the Docs from the Web GUI. The current behaviour is to open the main Docs site. The new behaviour is going to be to open a version-specific site instead. I'd say this is highly unexpected to everyone who's actually been using the link to open and browse the Docs.

A banner like that with a link to the main Docs site would be very welcome though 🙂.

@calmh
Copy link
Member

calmh commented Jan 22, 2022

Your expectation isn't suddenly wrong because of this change, the only difference is that you're not getting documentation for some other random version than the one you're running. I don't see this as a problem. I would rather think the common expectation for a user clicking a documentation link in a program is to get documentation for that program.

@tomasz1986
Copy link
Contributor

tomasz1986 commented Jan 22, 2022

This doesn't change the fact that it's still a significant change of the previous behaviour though. My opinion is that sudden changes with no indication of them happening are never good for user experience. As long as there's some kind of an indicator (i.e. the banner or similar), I've got no problem with it, but as it is right now, the redirection is done silently, and dissecting the URL seems to be the only way to even know about it.

@calmh
Copy link
Member

calmh commented Jan 22, 2022

Yeah I disagree on the significance of the change, but I filed syncthing/docs#718 so we don't forget nonetheless.

@acolomb
Copy link
Member Author

acolomb commented Jan 22, 2022

That was already recorded as one remaining task under #8107. And the suggestion with a version switcher is implemented on a technical level already, just needs some styling now.

I agree that nobody would probably care whether they see the main site or the one specific to their version. The latter is actually meant as an improvement. For you @tomasz1986 it won't make a difference since I assume you are always up to date or even running development builds. And actually older versions will not change behavior, it's only versions from when we merge this onwards where there will be a visible effect. Nobody suddenly gets to a different site.

@acolomb
Copy link
Member Author

acolomb commented Jan 22, 2022

Sorry @calmh about the syntax errors, must have been low on concentration while fixing the merge conflicts... Thanks for fixing them.

@calmh calmh merged commit 1af8757 into syncthing:main Jan 24, 2022
@acolomb acolomb deleted the docs-version-specific branch January 24, 2022 19:43
@calmh calmh added this to the v1.19.1 milestone Jan 27, 2022
@tomasz1986
Copy link
Contributor

Just a quick comment, but I believe the main footer link to the Docs was omitted in this one. Was it done on purpose or just by mistake?

@acolomb
Copy link
Member Author

acolomb commented Feb 3, 2022

That was on purpose. To have a link to the "project wide" site just like the forum and other web links. Makes sense to me at least, but it could be argued...

@tomasz1986
Copy link
Contributor

No strong opinion here, but to tell the truth, I did expect the link to be version specific, hence the surprise 😉.

@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Mar 10, 2023
@syncthing syncthing locked and limited conversation to collaborators Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gui: Links to documentation should point to matching version
4 participants