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

Implement a quiz-style installation instructions #11399

Merged
merged 11 commits into from
May 18, 2021

Conversation

dinever
Copy link
Contributor

@dinever dinever commented May 13, 2021

Hi,

This PR fixes #10353, see #10353 (comment)

The idea is to add a "quiz" style installation page. Inspired by CUDA and PyTorch:

CUDA PyTorch
Screen Shot 2021-05-13 at 2 23 14 AM Screen Shot 2021-05-13 at 2 23 23 AM

Showcase:

quiz

Some features:

  • Since this style is more scalable, I've added ARMv7, ppc64 and S390x architecture under Linux as well
  • If a row has only one option, it's automatically selected. e.g. Windows -> x86_64
    auto-select
  • The first row (OS) is automatically selected based on the user's OS

Would love to hear some feedback! Thanks!

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 13, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @dinever. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot requested review from afbjorklund and RA489 May 13, 2021 06:39
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 13, 2021
@dinever dinever force-pushed the installer-options branch from 5db2855 to 6808ccf Compare May 13, 2021 06:40
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2021
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@dinever dinever force-pushed the installer-options branch from 6808ccf to 487b4a3 Compare May 13, 2021 06:41
hack/jenkins/release_build_and_upload.sh Outdated Show resolved Hide resolved
@@ -23,7 +23,7 @@ $gray-800: #333 !default;
$gray-900: #222 !default;
$black: #000 !default;

$primary: #f2771a !default;
$primary: $mk-dark !default;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this primary color is actually used anywhere, and it's orange. So I changed it to our theme color which is the k8s blue.

@medyagh
Copy link
Member

medyagh commented May 13, 2021

@dinever thank you very much for taking this task ! I am so happy to see this page getting simplified,
btw the netlify site build is failing, https://app.netlify.com/sites/kubernetes-sigs-minikube/deploys/609ccc2bb8a7a400089227d0

@dinever dinever force-pushed the installer-options branch from bd422da to 606bf94 Compare May 13, 2021 20:07
@dinever dinever marked this pull request as draft May 13, 2021 20:12
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 13, 2021
@dinever dinever force-pushed the installer-options branch from 606bf94 to 109875d Compare May 13, 2021 20:18
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2021
@dinever dinever marked this pull request as ready for review May 13, 2021 20:32
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 13, 2021
@dinever
Copy link
Contributor Author

dinever commented May 13, 2021

@dinever thank you very much for taking this task ! I am so happy to see this page getting simplified,
btw the netlify site build is failing, https://app.netlify.com/sites/kubernetes-sigs-minikube/deploys/609ccc2bb8a7a400089227d0

Thanks! I've fixed the build failure.

I have to upgrade our Hugo version from 0.68.3 to 0.74.3since I need to use thestrings.Count` template function.

The site is now available for preview at: https://deploy-preview-11399--kubernetes-sigs-minikube.netlify.app/docs/start/

Thanks!

@k8s-ci-robot
Copy link
Contributor

@dinever: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2021
@dinever dinever force-pushed the installer-options branch from 109875d to 28915b0 Compare May 13, 2021 22:06
@medyagh
Copy link
Member

medyagh commented May 14, 2021

@dinever thank you very much this looks much much better ! Is it possible that we have a default case on first view ?
For example in case of Linux show x86 binary

And also we used to have an auto detection for Mac and Linux and windows.... Does this pr preserve that ?

@dinever
Copy link
Contributor Author

dinever commented May 14, 2021

@dinever thank you very much this looks much much better ! Is it possible that we have a default case on first view ?
For example in case of Linux show x86 binary

And also we used to have an auto detection for Mac and Linux and windows.... Does this pr preserve that ?

Thanks!

It auto-detects the OS and chooses it for the user. I've tested on Linux and macOS and the auto-detection worked as expected. Will test on Windows, too.

Is it possible that we have a default case on first view ?

Sounds good. I can make it go one step further:

  1. If user OS is Linux: default to Linux -> x86 -> binary
  2. If user OS is macOS: default to macOS -> x86 -> binary
  3. If user OS is Windows: default to Windows -> x86 -> .exe download
  4. If user OS can't be detected: default to Linux -> x86 -> binary

@dinever
Copy link
Contributor Author

dinever commented May 14, 2021

Hi @medyagh,

I've pushed the latest commit cc0d065 to make sure the first option of each row is auto-selected (OS->x86->binary download). In addition, I've confirmed that OS auto-detection works for all 3 platforms. This way we can minimize the number of clicks users need to perform.

Please see the following GIF for a quick demo, and feel to try it at https://deploy-preview-11399--kubernetes-sigs-minikube.netlify.app/docs/start/. Thanks a lot for the feedback!

installation

@medyagh
Copy link
Member

medyagh commented May 14, 2021

thank you very much @dinever it looks great,

only thing is, possible to make the whole Install Section have a different background color simmilar to old way ? a light green/blue-ish ?

Screen Shot 2021-05-14 at 11 37 49 AM

btw in another note, not related to this PR but I wish our Boxes were not too long
for example the gray box for minikube start becomes very very long

hack/jenkins/release_build_and_upload.sh Outdated Show resolved Hide resolved
netlify.toml Outdated Show resolved Hide resolved
Copy link
Member

@medyagh medyagh left a comment

Choose a reason for hiding this comment

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

do u mind also pasting screenshot for a user who has disabled JavaScript, will it still have an acceptable look in case of no javascript ?

site/static/js/quiz.js Outdated Show resolved Hide resolved
@medyagh
Copy link
Member

medyagh commented May 17, 2021

@dinever here is the screenshot without Javascript, could we make it in a way that a user without javascript can see the default page (even if it is ugly) ?
Screen Shot 2021-05-17 at 12 50 34 PM

@dinever
Copy link
Contributor Author

dinever commented May 18, 2021

Hi all,

Thanks for the feedback! I've pushed the latest commits:

  1. ed78279: Update Hugo version to the latest 0.83.1 since I needed some template functions.
  2. a7d8a06: Reuse the existing getUserOS from tabs.js
  3. d50e861: Wrap the installation section within a blue card:
    Screen Shot 2021-05-18 at 2 02 07 AM
  4. 120925e: Revert the changes in release_build_and_upload.sh
  5. d29d0a8: Make sure the page renders acceptable result when JS is disabled:
With JavaScript Without JavaScript or with browsers that don't support jQuery (IE6)
install install-without-js

Feel free to try the preview site at https://deploy-preview-11399--kubernetes-sigs-minikube.netlify.app/docs/start/.

Thanks a lot!

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dinever, medyagh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 18, 2021
@medyagh
Copy link
Member

medyagh commented May 18, 2021

thank you very much @dinever

@medyagh
Copy link
Member

medyagh commented May 18, 2021

looks amazing

@medyagh medyagh merged commit ea1defe into kubernetes:master May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the getting started / installation page look nice again
6 participants