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

[WIP] Revamp download page #592

Merged
merged 1 commit into from Apr 15, 2018
Merged

Conversation

Daniel15
Copy link
Member

@Daniel15 Daniel15 commented Aug 7, 2017

The "Linux" tab was getting a bit unwieldy given there's now several unofficial packages alongside our official Debian and RPM packages. It seems like tabs are the wrong UI element for this page now that it has grown, so I've replaced the tabs with a dropdown list:

This was inspired by the Let's Encrypt / EFF Certbot site (https://certbot.eff.org/), which has a similar dropdown. Design input would be appreciated.

In addition, I've added instructions for installing the RC and nightly releases. Currently, installation of RCs is undocumented, and nightly releases have their own separate page at https://yarnpkg.com/en/docs/nightly.

For RC and nightly releases, the instructions are slightly different. To handle this, I wrapped the version-specific instructions in DIVs with particular classes (install-only-stable for stable instructions, install-only-rc for RCs, and install-only-nightly for nightlies) and show the right one depending on the selection.

Test plan
Go to http://localhost:4000/lang/en/docs/install/, it defaults to your current OS
Go to http://localhost:4000/lang/en/docs/install/#asdasdasdasd (invalid OS in URL), it defaults to your current OS
Go to http://localhost:4000/lang/en/docs/install/#debian (OS but no version in URL), it shows Debian instructions
Go to http://localhost:4000/lang/en/docs/install/#debian-rc (OS and version in URL), it shows Debian RC instructions
Switch OS in dropdown, verify that correct instructions are shown
Switch version in dropdown, verify correct instructions are shown

@Haroenv
Copy link
Member

Haroenv commented Aug 7, 2017

The build failed:

[...]
3:58:11 AM: Liquid Exception: Could not locate the included file '_installations/alpine.md' in any of ["/opt/build/repo/lang/zh-Hans/docs"]. Ensure it exists in one of those directories and, if it is a symlink, does not point outside your site source. in /_layouts/pages/install.html
[...]

@Daniel15
Copy link
Member Author

Daniel15 commented Aug 7, 2017

Huh... Do I need to create the page for other languages? The English version (lang/en/docs/_installations/alpine.md) exists.

@Haroenv
Copy link
Member

Haroenv commented Aug 7, 2017

Looking at it now, it works locally, I'm asking a retry of the build with a cleared cache.

EDIT: it still fails even if it's a branch on this repository

@Haroenv
Copy link
Member

Haroenv commented Aug 7, 2017

I can't build the production version myself, because downloading from crowdin gives me this message:

error: Seems Crowdin server API URL is not valid. Please check the `base_url` parameter in the configuration file.
make[1]: *** [crowdin-download] Error 1
make: *** [build-production] Error 2

Haroenv added a commit that referenced this pull request Aug 7, 2017
testing around for cause of #592's failure
@Haroenv
Copy link
Member

Haroenv commented Aug 7, 2017

Not sure what caused it exactly, but a7ba2e4 can be deployed

@Daniel15 Daniel15 changed the title Revamp download page [WIP] Revamp download page Aug 7, 2017
@BYK
Copy link
Member

BYK commented Aug 7, 2017

In addition, I've added instructions for installing the RC and nightly releases.

<3

@Haroenv
Copy link
Member

Haroenv commented Aug 8, 2017

I'm wondering if those pages should be made manually on crowdin, so that the download works 🤔

@Daniel15
Copy link
Member Author

Daniel15 commented Aug 9, 2017

Yeah, I have no idea. cc @thejameskyle - Do you remember if we have to do anything special when new pages are added?

@jamiebuilds
Copy link
Contributor

jamiebuilds commented Aug 9, 2017

Something should be running this script, but it doesn't appear like anything is:

website/Makefile

Lines 31 to 32 in 43fec17

crowdin-upload: test-crowdin
@crowdin-cli upload sources --auto-update -b master

@Daniel15
Copy link
Member Author

Daniel15 commented Aug 9, 2017

@thejameskyle Does that only run on master? Some of these .md files aren't in master yet, they're just in this PR.

@jamiebuilds
Copy link
Contributor

Oh right... I think we may have never run into this scenario before (where something in crowdin is explicitly depending on something not in crowdin – normally files are totally independent of one another). You might just need to run that script manually.

@Haroenv
Copy link
Member

Haroenv commented Aug 9, 2017

When I do make crowdin-upload I get messages about the URL being wrong (after I set the API key)

$ export CROWDIN_API_KEY=xxx
$ make crowdin-upload
error: Seems Crowdin server API URL is not valid. Please check the `base_url` parameter in the configuration file.
make: *** [crowdin-upload] Error 1

base_url doesn't exist in the crowdin.yaml, but it does have base_path: ., would that be related?

@Daniel15
Copy link
Member Author

I promise I haven't forgotten about this... I'll come back to it and figure out the Crowdin thing. 😛

@mikemaccana
Copy link
Contributor

PS. I love treating each Linux distro as an OS in it's own right, with top level native packages.

@Daniel15
Copy link
Member Author

Yeah... The "Linux" tab was getting very large, and there's not enough room to add many more tabs.

@mikemaccana
Copy link
Contributor

Sure but 90% are using Debian/Ubuntu or RHEL/Fedora. Always can do 'other' for users of yggdrasil etc. 😉

@Haroenv
Copy link
Member

Haroenv commented Nov 7, 2017

@Daniel15, I wanted to try out the Crowdin upload script, but there are some conflict in the linux installations that I'm not sure which to keep, could you fix those conflicts? ❤️

@Daniel15
Copy link
Member Author

Sure, I'll try to take a look again soon.

@Haroenv
Copy link
Member

Haroenv commented Apr 15, 2018

Deploy preview for yarnpkg ready!

Built with commit 8132462

https://deploy-preview-592--yarnpkg.netlify.com

@Daniel15
Copy link
Member Author

Confirmed it's working: https://deploy-preview-592--yarnpkg.netlify.com/en/docs/install

I'm going to merge this seeing as it's been sitting around for 8 months with no objections. We can iterate on the design and make it nicer :)

@Daniel15 Daniel15 merged commit 1be87ed into yarnpkg:master Apr 15, 2018
@Haroenv
Copy link
Member

Haroenv commented Apr 15, 2018

How did you get it to work in the end @Daniel15?

@mikemaccana
Copy link
Contributor

mikemaccana commented Apr 16, 2018

Just wanted to say I really like the new design - it goes straight to the best way to deploy packages for your OS. Well done!

@Daniel15
Copy link
Member Author

Daniel15 commented May 1, 2018

How did you get it to work in the end @Daniel15?

@Haroenv I ran the CrowdIn sync command on my computer, using the same access token as the Netlify build uses.

@Daniel15 Daniel15 deleted the revamp-download branch May 1, 2018 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants