Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

chore(docs): improve version picker #15381

Closed

Conversation

petebacondarwin
Copy link
Contributor

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

chore

What is the current behavior? (You can also link to an open issue here)

Versions in the version picker are hard-coded into each build
This means that newer versions are never shown in older versions of the docs

What is the new behavior (if this is a feature change)?

Now we get the version list from the npm registry as needed

Does this PR introduce a breaking change?

Nope

Please check if the PR fulfills these requirements

Other information:

@Narretz
Copy link
Contributor

Narretz commented Nov 10, 2016

I don't particularly like it.

  • external provider for cross-origin introduces another point of failure out of our control
  • the whole version getting is fully asnyc, which is bad usability (and bad for people with bad connections). Not sure if it can be cached, either.
  • It adds comparably a lot of code

Since this will only expose the complete list on any versions that include this commit, why don't we just generate a version list (maybe json) on code.angularjs.org/snapshot and get that, additionally to the data in the constant NG_VERSIONS (see #15281)? We'd still have to update the nginx config so it's reachable from docs.angularjs.org. This would eliminate the cross-origin proviider, and the need for so much code.

@petebacondarwin
Copy link
Contributor Author

I agree about the CORS provider, and using a file that is stored on our own server with CORS enabled would be much better.

The increase in code size is a fair point and if we generate our own files then we can prepare them so that we don't need to do any semver parsing etc.

I am not convinced that asynchronicity is an issue. The requests are made in the background and do not block the user from accessing the docs app and navigating between API pages. It seems unlikely that the user will load the page and immediately try to use the version dropdown before it has been filled with data. In fact putting the data into script files that must be loaded before we can bootstrap Angular is probably going to impact the usability more, since it adds a delay before the entire app can be used.

I'll try again.

@Narretz
Copy link
Contributor

Narretz commented Nov 10, 2016

We don't know how many people change the docs version instantly. But I wouldn't underestimate the number of people who still use older versions, and just go to docs.angularjs.org and change their version. This was probably even more of an issue when we were defaulting to snapshot instead of latest stable. It's true about the payload, but you didn't remove the NG_VERSIONS creation, so I thought why not use it.

Some more feedback would be good, but if we go full async then we should put the current version instantly into the select, and provide a loading indicator when the select has been opened and the versions haven't arrived yet.

@petebacondarwin
Copy link
Contributor Author

Closing in favour of #15385

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants