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

[Redesign] Update installation instructions #8690

Merged
merged 2 commits into from
Jul 22, 2021

Conversation

loic-sharma
Copy link
Contributor

@loic-sharma loic-sharma commented Jul 16, 2021

Addresses #8602

Screenshots

Default screenshot...

image

Package with long ID:

image

Installation instructions with banners below...

image

image

Multi-line installation instructions...

image

image

.NET Tools...

image

⚠ .NET Template...

image

⚠ NOTE: There is only one package manager that supports .NET Templates today. The UI has a dropdown with only a single option.

Mobile...

FYI the installation instructions are sideways scrollable. You can swipe (or is it pan?) to see the text:

image

image

sophiamfavro and others added 2 commits July 7, 2021 14:35
@loic-sharma loic-sharma force-pushed the loshar-installation-instructions branch from fbe830c to c41185f Compare July 16, 2021 21:41
@loic-sharma loic-sharma marked this pull request as ready for review July 16, 2021 21:42
@loic-sharma loic-sharma requested a review from a team as a code owner July 16, 2021 21:42
@joelverhagen
Copy link
Member

Have you tested multi-line commands (<PackageReference> for dev dependency) or long commands (long ID/version)?

@loic-sharma
Copy link
Contributor Author

@joelverhagen Yup, I added a bunch of screenshots for a bunch of different scenarios including multi-line, long package IDs, mobile, etc... Let me know if I missed any fun cases!

@joelverhagen
Copy link
Member

These two lines look different length, i.e. it looks like the command text is not vertically centered. Intentional?
image

@loic-sharma
Copy link
Contributor Author

loic-sharma commented Jul 16, 2021

@joelverhagen Yup that's intentional. It is added when there is an alert below for spacing, but the spacing is equal when there is no alert below. It looks a bit cramped if we remove that spacing below:

image

/cc @jcjiang

@loic-sharma
Copy link
Contributor Author

@jcjiang confirmed they approve of the extra padding on the installation instructions if they have a banner below.

@loic-sharma loic-sharma merged commit 47a2879 into dev Jul 22, 2021
@loic-sharma loic-sharma deleted the loshar-installation-instructions branch July 22, 2021 17:08
});

// Used to switch installation instructions when a new package manager is selected
function updatePackageManager(newPackageManagerId, updateSelector) {
Copy link
Contributor

@zhhyu zhhyu Jul 22, 2021

Choose a reason for hiding this comment

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

I see that "updateSelector" is mainly used when restoring the preferred selection from the local storage. So we can set packageManagerSelector[0].value = preferredPackageManagerId; after line 63 updatePackageManager(preferredPackageManagerId, true);, and then there is no need to keep the argument "updateSelector" any more.

I don't feel strong, but this may be more clean and easier to follow.

loic-sharma added a commit that referenced this pull request Aug 27, 2021
Reverts the installation instructions added in #8690.

Addresses #8762
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.

4 participants