feat: Redesign workspaces page by kylecarbs · Pull Request #1450 · coder/coder

kylecarbs · 2022-05-14T03:07:15Z

As part of redesigning the workspaces page, I updated the theme to dark mode and removed overrides resolving #1449. There's a lot of work that can be done here to improve, but this is a good start!

I've checked all pages to ensure nothing is bugged/broken, and it all looks good!

A text input will be added in the future to allow for filtering. This will work similarly to GitHub's issue filters: owner:kylecarbs, status:running, is:outdated, etc.

I proposed initially that workspaces be nested under templates, but that provided a restrictive view for admins. Admins and managers should get a full view of their Coder deployment from the workspaces page, which they now can!

Todo

The navbar was unnecessarily large before, which made the UI feel a bit bloaty from my perspective.

code-asher

Looks good to me so far! All the pages look great but I think something needs to be updated for the stories since they are all throwing errors.

code-asher · 2022-05-14T16:25:25Z

+        content: `" "`,
+        bottom: 0,
+        left: theme.spacing(3),
+        background: "#C16800",


Is there a good theme color that could go here in place of the hardcoded color? Like secondary or something maybe.

code-asher · 2022-05-14T17:20:06Z

    proxy: {
      "/api": {
-        target: "http://localhost:3000",
+        target: "https://dev.coder.com",


Should this be an environment variable or something so we can still develop locally?

greyscaled · 2022-05-14T17:32:04Z

@code-asher @kylecarbs

I think something needs to be updated for the stories since they are all throwing errors.

The theme provider for storybook needs to be set in .storybook/preview.js --> we can modify the below to only accept dark theme. I didn't test that locally, but it's a start as to where to look.

coder/site/.storybook/preview.js

Lines 7 to 17 in dbd5b4a

    
           import { dark, light } from "../src/theme" 
        
           import "../src/theme/globalFonts" 
        
           const providerFn = ({ children, theme }) => ( 
        
             <ThemeProvider theme={theme}> 
        
               <CssBaseline /> 
        
               {children} 
        
             </ThemeProvider> 
        
           ) 
        
           addDecorator(withThemes(null, [light, dark], { providerFn }))

greyscaled · 2022-05-14T17:33:16Z

  },
  "dependencies": {
-    "@fontsource/fira-code": "4.5.9",
+    "@fontsource/ibm-plex-mono": "^4.5.9",


Suggestion: Can you pin this dependency to 4.5.9? You run the upgrade command to do it:

cd site yarn upgrade @fontsource/ibm-plex-mono@4.5.9

@vapurrmaid I notice we have yarn.lock - what's the thought process behind pinning dependencies?

I'll answer those two questions separately:

Why do we want them pinned

vs

Why pin in package.json

For the former: We have a RFC and guide covering, but the TL:DR is more incremental upgrades w depend-a-bot
For the latter: I don't know if having them unpinned interferes with depend-a-bot or any other tool, but simply for precision. I shouldn't have to hunt down the actual version in yarn.lock to answer what version of X we're on.

Thanks, will look at the guide!

greyscaled · 2022-05-14T17:44:16Z

+              <TableCell>Name</TableCell>
+              <TableCell>Template</TableCell>
+              <TableCell>Version</TableCell>
+              <TableCell>Last Built</TableCell>
+              <TableCell>Status</TableCell>


Suggestion: All of these should be defined in Language, including the <Button> text above

Where did the Language convention come from? Have we thought about using react-i18next?

Great question!

i18n was not implemented in v1, and will be an eventual need for v2. We haven't yet planned for when but we know it will be an effort that we will eventually take on. In the meantime, Language is a simple way of using primitives to begin resourcing strings for what's next. The idea wasn't to commit to a framework or have to solve that problem, but ensure that we're organized and ready to for when we do.

As a bonus, we can share Language in test like this:

coder/site/src/pages/LoginPage/LoginPage.test.tsx

Lines 21 to 27 in 26b04cc

it("renders the sign-in form", async () => {

// When

render(<LoginPage />)

// Then

await screen.findByText(Language.passwordSignIn)

})

Great, make sense to me! Thanks for explaining.

greyscaled · 2022-05-14T17:57:00Z

Praise I really like the Status column, it's very nice 🎨

BrunoQuaresma · 2022-05-16T13:17:38Z

@kylecarbs I assigned this ticket #766 to you since this work looks to be related to it.

Kira-Pilot · 2022-05-16T13:53:24Z

I noticed we're keeping some overrides in theme/overrides.ts - is removing these work leftover that should be completed at a later time, or do we need to keep this stuff?

kylecarbs · 2022-05-16T13:56:28Z

@Kira-Pilot those are things I added specifically in this PR to adjust some stylistic things. material-ui mostly worked for our needs, but I just tweaked a few smaller visual items.

presleyp · 2022-05-16T15:00:05Z

@@ -0,0 +1,32 @@
+export const getTimeSince = (date: Date): string => {


Should we use a library for this time stuff instead? I've used moment.js and day.js in the past

Yup, I implemented @vapurrmaid's suggestion of using day.js!

I made this suggestion above: #1450 (comment) which is prior art from v1. I'm quite happy with dayjs from that experience, and I know it's a small library.

Kira-Pilot · 2022-05-16T15:47:49Z

@kylecarbs I am having a hard time running this locally using ./develop.sh. @presleyp was struggling as well. Any idea what we might be missing?

BrunoQuaresma · 2022-05-16T17:37:26Z

Doing some QA I was not able to create a new workspace from the UI:

Is it expected?

kylecarbs · 2022-05-16T17:49:15Z

Yup! The create workspace page is included in #801

code-asher

I like the default look of the field input labels. The dialogs looked a bit better before IMO but definitely still usable (I like the reset password dialog though).

The margins component might need a background color removed or something, right now it has white text on a gray background https://www.chromatic.com/test?appId=624de63c6aacee003aa84340&id=62828332ad84ca004a3e0e03

Very handy we can see all the different workspace states in Chromatic!

Edit: this one seemed a bit strange to me as well, is that a disabled button? https://www.chromatic.com/test?appId=624de63c6aacee003aa84340&id=62828332ad84ca004a3e0ded
Not sure if this matters but should this be white? https://www.chromatic.com/test?appId=624de63c6aacee003aa84340&id=62828332ad84ca004a3e0dfd

greyscaled

One quick question I thought I'd submit review for right away since it's simple.

Following up with other items shortly.

greyscaled · 2022-05-16T18:54:22Z

@@ -1,20 +1,17 @@
 import CssBaseline from "@material-ui/core/CssBaseline"
 import ThemeProvider from "@material-ui/styles/ThemeProvider"
-import { withThemes } from "@react-theming/storybook-addon"


Question(if-minor): Are we able to remove this dependency now "@react-theming/storybook-addon": "1.1.5", ?

I think so!

Good catch.

greyscaled · 2022-05-16T19:02:10Z

+
+export const Language = {
+  createButton: "Create Workspace",
+  emptyView: "so you can check out your repositories, edit your source code, and build and test your software.",


cc @khorne3 for review

https://www.chromatic.com/component?appId=624de63c6aacee003aa84340&csfId=pages-workspacespageview--empty&buildNumber=1611&k=62828332ad84ca004a3e0e29-1200-interactive-true&h=9&b=-4

kylecarbs · 2022-05-16T19:27:50Z

@code-asher all have been fixed! That weird button is actually a loading button, and looked weird before!

presleyp · 2022-05-16T19:39:14Z

 const Template: Story = (args) => (
  <Margins {...args}>
-    <div style={{ width: "100%", background: "lightgrey" }}>Here is some content that will not get too wide!</div>
+    <div style={{ width: "100%" }}>Here is some content that will not get too wide!</div>


This still needs a background color - it proves that the margins are working

Ahh good catch! I'll make it black.

presleyp · 2022-05-16T19:48:32Z

+import { WorkspacesPageView } from "./WorkspacesPageView"
+
+const WorkspacesPage: React.FC = () => {
+  const [workspacesState] = useMachine(workspacesMachine)


This reminds me that now that I'm doing a bunch of polling on the workspace page, I can and should useMachine for that too.

Oh interesting. I never thought of that, but I suppose it makes sense.

presleyp

Looks good, I have a couple important but small comments about the status bar and then some stylistic things you can decide on

kylecarbs · 2022-05-16T21:52:41Z

Thank you for the reviews everyone 🥳🥳🥳🥳🥳

@vapurrmaid I see you're pending on UI for Storybook. I'm going to merge this in, but I'm extremely happy to make any changes post!

misskniss · 2022-05-17T19:02:04Z

fixes #699
fixes #766

kylecarbs added 6 commits May 13, 2022 22:21

feat: Improve navbar to be more compact

1a667d9

The navbar was unnecessarily large before, which made the UI feel a bit bloaty from my perspective.

Attempt to remove overrides

dc47ae0

Update theme

c34ad45

Add text field

104b1d9

Merge branch 'main' into workspacepage

52a4e76

Update theme to dark!

83397e0

kylecarbs requested review from a team and presleyp as code owners May 14, 2022 03:07

kylecarbs added 2 commits May 14, 2022 03:17

Fix import ordering

d19c124

Fix page location

db820ce

code-asher reviewed May 14, 2022

View reviewed changes

greyscaled requested a review from Kira-Pilot May 14, 2022 17:35

greyscaled reviewed May 14, 2022

View reviewed changes

kylecarbs mentioned this pull request May 16, 2022

fix: Remove CSS overrides for material-ui #1444

Closed

Kira-Pilot reviewed May 16, 2022

View reviewed changes

Comment thread site/src/app.tsx

BrunoQuaresma mentioned this pull request May 16, 2022

Frontend Workspace Read - List #766

Closed

7 tasks

BrunoQuaresma assigned kylecarbs May 16, 2022

kylecarbs mentioned this pull request May 16, 2022

Add /workspaces to allow for filtered and paginated querying #1477

Closed

kylecarbs added 2 commits May 16, 2022 14:21

Fix requested changes

851111b

Add storybook for workspaces page view

190b7d1

presleyp reviewed May 16, 2022

View reviewed changes

kylecarbs added 3 commits May 16, 2022 16:03

Add empty view

cbc1015

Add tests for empty view

169d733

Remove templates page

53781e9

Kira-Pilot approved these changes May 16, 2022

View reviewed changes

code-asher reviewed May 16, 2022

View reviewed changes

Comment thread site/src/util/time.test.ts Outdated

Comment thread site/src/pages/WorkspacesPage/WorkspacesPageView.tsx Outdated

greyscaled reviewed May 16, 2022

View reviewed changes

Remove time.ts

82cdace

kylecarbs added 2 commits May 16, 2022 19:27

Remove dep

6044d91

Merge branch 'main' into workspacepage

bd50c9f

presleyp reviewed May 16, 2022

View reviewed changes

Add background color to margins

191c9d7

presleyp reviewed May 16, 2022

View reviewed changes

Comment thread site/src/pages/WorkspacesPage/WorkspacesPageView.tsx Outdated

code-asher approved these changes May 16, 2022

View reviewed changes

presleyp reviewed May 16, 2022

View reviewed changes

kylecarbs force-pushed the workspacepage branch from a865ebe to e9778f0 Compare May 16, 2022 20:20

Merge status checking from workspace page

fccacfe

kylecarbs force-pushed the workspacepage branch from e9778f0 to fccacfe Compare May 16, 2022 20:30

presleyp approved these changes May 16, 2022

View reviewed changes

Comment thread site/src/components/WorkspaceStatusBar/WorkspaceStatusBar.tsx

Comment thread site/src/pages/WorkspacesPage/WorkspacesPageView.tsx Outdated

Comment thread site/src/xServices/workspaces/workspacesXService.ts

kylecarbs added 2 commits May 16, 2022 20:54

Fix requested changes

f87c8e7

Fix workspace status tests

a058b8c

kylecarbs merged commit 22ec366 into main May 16, 2022

kylecarbs deleted the workspacepage branch May 16, 2022 21:52

This was referenced May 17, 2022

Create user identities table for OICD / Git providers #799

Closed

Set the theme to dark mode throughout the app #699

Closed

Remove theme overrides #1449

Closed

greyscaled mentioned this pull request May 20, 2022

feat: WorkspaceSection action #1623

Merged

	it("renders the sign-in form", async () => {
	// When
	render(<LoginPage />)

	// Then
	await screen.findByText(Language.passwordSignIn)
	})

		@@ -0,0 +1,32 @@
		export const getTimeSince = (date: Date): string => {

Uh oh!

Conversation

kylecarbs commented May 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greyscaled commented May 14, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greyscaled May 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greyscaled commented May 14, 2022

Uh oh!

Uh oh!

BrunoQuaresma commented May 16, 2022

Uh oh!

Kira-Pilot commented May 16, 2022

Uh oh!

kylecarbs commented May 16, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greyscaled May 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kira-Pilot commented May 16, 2022

Uh oh!

BrunoQuaresma commented May 16, 2022

Uh oh!

kylecarbs commented May 16, 2022

Uh oh!

code-asher left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

greyscaled left a comment

Choose a reason for hiding this comment

Uh oh!

greyscaled May 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

kylecarbs commented May 14, 2022 •

edited

Loading

greyscaled May 16, 2022 •

edited

Loading

greyscaled May 16, 2022 •

edited

Loading

code-asher left a comment •

edited

Loading

greyscaled May 16, 2022 •

edited

Loading

misskniss commented May 17, 2022 •

edited

Loading