[UI] Add new themes to Packages List view page #9852
Conversation
| @@ -4,7 +4,7 @@ | |||
| value="@(String.IsNullOrEmpty(ViewBag.SearchTerm) ? "" : ViewBag.SearchTerm)" | |||
| @(ViewBag.AutofocusSearch == true ? "autofocus" : string.Empty) /> | |||
| <span class="input-group-btn"> | |||
| <button class="btn btn-default btn-brand btn-search" type="submit" | |||
There was a problem hiding this comment.
btn-default adds styles that are not needed, removing it to reduce the overriding of css properties
| <label for="prerel-checkbox" aria-label="Options: Include prerelease">Include prerelease</label> | ||
| <label class="brand-checkbox" aria-label="Options: Include prerelease"> | ||
| <input id="prerel-checkbox" type="checkbox" checked="@Model.IncludePrerelease"> | ||
| <span> |
There was a problem hiding this comment.
Added spans so we can select the text from CSS, otherwise is rendered just as text
|
Please add link to main tracking issue and if there's any spec. |
| @@ -926,6 +926,10 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version | |||
|
|
|||
| // Load all packages with the ID. | |||
| var allVersions = _packageService.FindPackagesById(id, includePackageRegistration: true); | |||
| includePackageRegistration: true, | |||
There was a problem hiding this comment.
need to remove this too, bad rebase
| @@ -49,6 +49,9 @@ p { | |||
| line-height: 22px; | |||
| /* 137.5% */ | |||
| } | |||
| label { | |||
| color: var(--neutralForeground1Rest); | |||
There was a problem hiding this comment.
Since we are using less, what's the benefit of using var(...)? Can't we leave the explicit color and get a bit more legacy support that way?
There was a problem hiding this comment.
Talked with Joel offline and we decided to continue with CSS variables, IE is the main concern since it doesn't support them, after merging into dev we will verify that the page is usable I'll track it here #9875
There was a problem hiding this comment.
Do we use var in our CSS at all? I see it's supported on most recent browser versions but it seems strange to use this feature along with LESS transpiler, i.e. we are already "compiling" our CSS so why not sub the variables for real values for a bit of back compat. I don't feel super strongly but I'd like to at least understand the motivation of using this CSS feature for I believe the first time.
Do you plan on merging the PR as-is with the a11y errors or are you blocking on that?
There was a problem hiding this comment.
This looks good to me, pending @joelverhagen's clarifications.
* Add new theme to Package List page
* Add new theme to Package List page
* Add new theme to Package List page
* Add new theme to Package List page
* Add new theme to Package List page
* Add new theme to Package List page
* Add new theme to Package List page
* Add new theme to Package List page
Addresses #9854
Figma: https://www.figma.com/file/ZaAD9rDujh8JDkRF4q9Fd8/NuGet---Simple-theme-updates?type=design&node-id=61%3A9109&mode=design&t=Urj8iIoXc4thBO3E-1
Spec: https://github.com/NuGet/Engineering/pull/5209
Description
These PR add's colors/theme for the Package List view, for this page we added:
Dark Theme
Light Theme
Transparent button
States
Checkbox, Switch and TreeView
The icon button uses the new transparent button so the styles are the same.
Dark
Light
Radio button
Badges
Accessibility errors
Links color are not accessible enough
I'll contact designers again to ask for guidance here. There are two options, add an underline for these kinds of links or change the color of hyperlinks
