[dashboard] Update design of class settings #11435
Conversation
|
/werft run 👍 started the job as gitpod-build-fo-settings-classes.2 |
roboquat
added
release-note-none
and removed
do-not-merge/work-in-progress
do-not-merge/release-note-label-needed
labels
|
Looking at this now! 👀 |
There was a problem hiding this comment.
Thanks for making this change, @Furisto! 🔮
Left a couple of suggestions, nitpicks, and questions below.
Approving to unblock merging but holding to let someone from @gitpod-io/engineering-webapp take a closer look at the code changes.
/hold
| selected={workspaceClass === "g1-standard"} | ||
| onClick={() => actuallySetWorkspaceClass("g1-standard")} | ||
| category="GENERAL PURPOSE" | ||
| friendlyName="Standard" |
There was a problem hiding this comment.
suggestion(non-blocking): Shall we use kebab-case (all lowercase) here?
| BEFORE | AFTER (kebab-case) |
|---|---|
 |
 |
There was a problem hiding this comment.
I prefer the current way. The category and the description start with an upper case letter and the class starting with a lower case letter looks out of place to me.
There was a problem hiding this comment.
Any option would suffice for now.
Looping in @jldec as this will affect usage page pages which will surface the class.
| </svg> | ||
| </div> | ||
| </SelectableCardSolid> | ||
| <WorkspaceClass |
There was a problem hiding this comment.
question: Is the SelectableCardSolid not flexible enough for what we need here? I'd love us to reuse the same component and maintain styles and interaction in one place. Haven't checked in depth but would that require us to update the SelectableCardSolid component?
There was a problem hiding this comment.
I found it nicer to encapsulate everything that is needed for a workspace class in one place, but I am happy to explore using SelectableCardSolid instead.
There was a problem hiding this comment.
fyi: Opened follow-up issue #11472 to track this.
Furisto
force-pushed
the
fo/settings-classes
branch
from
4cdb19b to
15141e4
Compare
|
/unhold |
Description
Update design of class settings
Related Issue(s)
Fixes #11358
How to test
Look at the settings page
Release Notes
Documentation
Werft options: