-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Introduce new version designs #15397
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
Conversation
started the job as gitpod-build-ft-ide-versions-ui.1 because the annotations in the pull request description changed |
Currently what I have done with the styles in the preview environment: cc @gtsiolis |
@@ -176,42 +176,40 @@ function renderIdeOption( | |||
version: IDEOption["imageVersion"], | |||
onSelect: () => void, | |||
): JSX.Element { | |||
const label = option.type === "desktop" ? "" : option.type; | |||
const shouldShowOptionType = option.type !== "desktop" || option.title === "VS Code"; // Force show of "Desktop" in the list for VS Code Desktop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
taking a note on hardcoding, let's go with it for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, we could add something like forceShowType
in https://github.com/gitpod-io/gitpod/blob/main/install/installer/pkg/components/ide-service/ide_config_configmap.go to VS Code Desktop to remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
/hole
since commits should be squashed
Looking at this now! 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this change, @filiptronicek! 🔮 🧙
Left some minor comments below about spacing and colors. The font stack comments can be resolved separately. Let me know if anything looks unclear how to proceed. 🛹
@@ -4,12 +4,14 @@ | |||
* See License.AGPL.txt in the project root for license information. | |||
*/ | |||
|
|||
export type PillType = "info" | "warn" | "success"; | |||
export type PillType = "info" | "warn" | "success" | "warnStrong" | "subtle"; | |||
|
|||
const PillClsMap: Record<PillType, string> = { | |||
info: "bg-blue-50 text-blue-500 dark:bg-blue-500 dark:text-blue-100", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: Going with the same subtle (neutral) label could be nice with the upcoming changes in #15389, see relevant designs in #15287 (comment). Cc @svenefftinge
Alternatively, using the info variant could be also a good way forward. But in any case, going with the neutral and improving the label designs (colors) (#15218) in the future sounds good.
Using neutral label variant | Using info label variant |
---|---|
![]() |
![]() |
@filiptronicek I accidentally missed uploading these BEFORE / AFTER screenshots to clarify some of the suggestions in the UX review above in #15397 (review). Let me know if anything looks unclear, and let's open follow-up issues for anything that can be tackled in next iterations. 🛹
|
a43c41c
to
a5bb7e3
Compare
Description
Related Issue(s)
Fixes #15194
How to test
Release Notes
Documentation
Werft options:
If enabled this will build
install/preview
Valid options are
all
,workspace
,webapp
,ide
,jetbrains
,vscode
,ssh