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.

Sign up for GitHub

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

Jump to bottom

Configure workspace classes from dashboard #11038

Merged
merged 3 commits into from
Jul 6, 2022
Merged

Configure workspace classes from dashboard #11038

merged 3 commits into from
Jul 6, 2022

Conversation

Furisto
Copy link
Member

@Furisto Furisto commented Jun 30, 2022 •
edited
Loading

Description

Add dialog for selecting workspace classes to preferences. Note that the current icons and class names are placeholders. This PR is purely for setting up the logic in the frontend.

Classes_Dialog

Related Issue(s)

Fixes ##10843

How to test

  • Open dashboard and go to Settings -> Preferences
  • Set your workspace class
  • Switch to another submenu and switch back -> your changes should have been persisted
  • In the user table, additionalData should have a workspaceClasses entry

Release Notes

NONE

Werft options:

  • /werft with-preview

@roboquat roboquat added size/L and removed size/M labels Jun 30, 2022
@geropl geropl self-assigned this Jul 1, 2022
@Furisto Furisto marked this pull request as ready for review July 1, 2022 14:53
@Furisto Furisto requested a review from a team July 1, 2022 14:53
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jul 1, 2022
gtsiolis
gtsiolis reviewed Jul 4, 2022
View reviewed changes
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Thanks for adding this, @Furisto! ✨

Left two comments based on the recent discussions[1][2] (internal) about the UX of this feature, to avoid merging this while using the same icons from the theme preference section. 🏁

Cc @atduarte

components/dashboard/src/settings/selectClass.tsx
Comment on lines +51 to +65
<SelectableCardSolid
className="w-36 h-32"
title="Standard"
selected={workspaceClass === "standard"}
onClick={() => actuallySetWorkspaceClass("standard")}
>
<div className="flex-grow flex items-end p-1">
<svg width="112" height="64" fill="none" xmlns="http://www.w3.org/2000/svg">
<path
d="M0 8a8 8 0 0 1 8-8h16a8 8 0 1 1 0 16H8a8 8 0 0 1-8-8ZM0 32a8 8 0 0 1 8-8h16a8 8 0 1 1 0 16H8a8 8 0 0 1-8-8ZM0 56a8 8 0 0 1 8-8h16a8 8 0 1 1 0 16H8a8 8 0 0 1-8-8ZM40 6a6 6 0 0 1 6-6h60a6 6 0 0 1 6 6v28a6 6 0 0 1-6 6H46a6 6 0 0 1-6-6V6Z"
fill="#D6D3D1"
/>
</svg>
</div>
</SelectableCardSolid>
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: Friendly reminder to avoid merging this with SVG icons used for the theme preference. ❗

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi George, the current icons are placeholders. These icons will not be visible to the user at the moment (will be hidden behind a feature flag as the current design and workspace class names have not been finalized). This is just for setting up the logic in the frontend, so that I can continue with the other changes in the backend.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok! Thanks for clarifying, @Furisto! 🙏

Feel free to ignore these two comments[1][2] then.

components/dashboard/src/settings/selectClass.tsx
Comment on lines +51 to +65
<SelectableCardSolid
className="w-36 h-32"
title="Standard"
selected={workspaceClass === "standard"}
onClick={() => actuallySetWorkspaceClass("standard")}
>
<div className="flex-grow flex items-end p-1">
<svg width="112" height="64" fill="none" xmlns="http://www.w3.org/2000/svg">
<path
d="M0 8a8 8 0 0 1 8-8h16a8 8 0 1 1 0 16H8a8 8 0 0 1-8-8ZM0 32a8 8 0 0 1 8-8h16a8 8 0 1 1 0 16H8a8 8 0 0 1-8-8ZM0 56a8 8 0 0 1 8-8h16a8 8 0 1 1 0 16H8a8 8 0 0 1-8-8ZM40 6a6 6 0 0 1 6-6h60a6 6 0 0 1 6 6v28a6 6 0 0 1-6 6H46a6 6 0 0 1-6-6V6Z"
fill="#D6D3D1"
/>
</svg>
</div>
</SelectableCardSolid>
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Re-posting some updated design specs after taking into account some feedback, see relevant discussion (internal) and design specs. Cc @atduarte

Option 🅰️ Option 🅱️
WorkspaceClasses WorkspaceClasses-1

@geropl
Copy link
Member

geropl commented Jul 5, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-fo-classes-fe.7
(with .werft/ from main)

geropl
geropl approved these changes Jul 5, 2022
View reviewed changes
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

Code LGTM(*), tested and works 👍

As discussed offline with @Furisto we agreed that:

  • it would be nice to have be able to configure the concrete strings for workspace classes in a central place, and load them from there
  • as this is currently feature flagged, merging the current state is ok to move forward
  • before we remove the feature flag, we should have such a config mechanism + APIs to retrieve them

@roboquat roboquat merged commit f157a02 into main Jul 6, 2022
@roboquat roboquat deleted the fo/classes-fe branch July 6, 2022 05:43
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtsiolis gtsiolis gtsiolis left review comments

@geropl geropl geropl approved these changes

Assignees

@geropl geropl

Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production feature: workspace classes release-note-none size/L team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Furisto @geropl @gtsiolis @roboquat