[dashboard] misc dashboard improvements #3496

svenefftinge · 2021-03-18T21:32:52Z

opening workspaces in new tabs (fixes [dashboard] Allow CMD+Click to open workspaces in a new tab. #3492)
allow stopping running workspaces
deleting workspaces works
modal are closed with ESC or click outside of modal (fixes [dashboard] Close modals when typing esc key and clicking outside the modal #3493)
modals can handle enter key.

svenefftinge · 2021-03-19T07:50:23Z

components/dashboard/src/workspaces/Workspaces.tsx

@@ -89,7 +89,7 @@ export class Workspaces extends React.Component<WorkspacesProps, WorkspacesState
                            <div className="w-3/12">Name</div>
                            <div className="w-4/12">Context</div>
                            <div className="w-2/12">Pending Changes</div>
-                            <div className="w-2/12">Last Active</div>
+                            <div className="w-2/12">Last Start</div>


@gtsiolis I changed this to Last Start because that is what the time really shows. Last Active was confusing because for running workspaces it seems they should be active just now.

I was tempted to suggest to revert this but let's leave this as is for now. Seems more accurate indeed!

gtsiolis · 2021-03-19T08:24:06Z

/werft run

👍 started the job as gitpod-build-se-misc-dashboard.2

AlexTugarev · 2021-03-19T08:47:08Z

components/dashboard/src/components/Modal.tsx

    if (!props.visible) {
        return null;
    }
-    setTimeout(() => window.addEventListener('click', props.onClose, { once: true }), 0);
+    setTimeout(() => {


I'd suggest to use useEffect hook instead in order to guarantee that the event listeners are received by the visible modals only.

This is what I did in parallel.

const handler = (e: MouseEvent) => { // ... } useEffect(() => { if (!props.visible) { window.removeEventListener("click", handler) return; } window.addEventListener("click", handler) // do something on unmount return () => { }; }, [props.visible]);

This hook's invocation depends on props.visible change, which gives us control of the event handlers.

OTOH, given the setTimeout solution, a <Modal visible={false}></Modal> would already register the event handler.

AlexTugarev · 2021-03-19T08:48:27Z

components/dashboard/src/components/ContextMenu.tsx

-            onClick: () => {
-                e.onClick && e.onClick();
+            onClick: (event: MouseEvent) => {
+                e.onClick && e.onClick(event);


AlexTugarev · 2021-03-19T09:13:27Z

components/dashboard/src/components/Modal.tsx

    return (
-        <div className="fixed top-0 left-0 bg-black bg-opacity-70 z-50 w-screen h-screen" >
+        <div className="fixed top-0 left-0 bg-black bg-opacity-70 z-50 w-screen h-screen" onClick={close}>


I believe, onClick={close} here will trigger close even if you click-select anything within the modal.

It does not, because I stop.propagation below

gtsiolis

Alright, this is shaping up! 🌟

Here're some more outstanding issues, feel free to resolve here or a new PR. Thanks @svenefftinge!

When searching and focus is on the search text field, on ESC clear the search input.
Change placeholder text Search Workspace to Search Workspaces.
Clicking Open in the more actions dropdown for stopped workspaces does not work.
When manually stopping a workspace redirect to dashboard.
Persist top menu state on Workspaces and Settings
Could we apply a subtle hover transition effect on everything? See also relevant discussion.
When clicking on Workspaces the top menu always move to Active workspaces.
What do you think of adding animate-pulse on kumquar workspaces status indicator for starting and stopping workspaces?
Let's also change the font weight for more actions dropdown items to font-medium
Let's change the width of the dropdown of the more actions to w-48. This will also use the same width as the side menu. Also, let's change the dropdown spacing from space-y-2 to space-y-1 and py-2 to py-1.
What do you think of removing the responsive layout of the table? Is this easily turned off? If not, feel free to ignore for now.
Can we also remove the responsive layout of the top menu and profile menu?
The more actions dropdown is missing a top margin of 8px from the more action button.
Workspace more actions button should be visible only on hover. See similar approach in [dashboard] Re-implement running workspace selector for new dashboard #3464 (comment).
The filter prefix is nice but everything up there on the top right is a filter. I still think Status prefix is better. Feel free to leave this as is for now.

gtsiolis · 2021-03-19T08:32:59Z

components/dashboard/src/components/Modal.tsx

+    setTimeout(() => {
+        const keyHandler = (k: globalThis.KeyboardEvent) => {
+            if (!k.bubbles) {
+                if (k.key === 'Escape') {


issue: Modals do not close on ESC. 💭

fixed, the predicate for the capturing phase was wrong

gtsiolis · 2021-03-19T08:34:18Z

components/dashboard/src/workspaces/Workspaces.tsx

@@ -89,7 +89,7 @@ export class Workspaces extends React.Component<WorkspacesProps, WorkspacesState
                            <div className="w-3/12">Name</div>
                            <div className="w-4/12">Context</div>
                            <div className="w-2/12">Pending Changes</div>
-                            <div className="w-2/12">Last Active</div>
+                            <div className="w-2/12">Last Start</div>


I was tempted to suggest to revert this but let's leave this as is for now. Seems more accurate indeed!

gtsiolis · 2021-03-19T08:36:10Z

components/dashboard/src/workspaces/Workspaces.tsx

@@ -89,7 +89,7 @@ export class Workspaces extends React.Component<WorkspacesProps, WorkspacesState
                            <div className="w-3/12">Name</div>
                            <div className="w-4/12">Context</div>
                            <div className="w-2/12">Pending Changes</div>
-                            <div className="w-2/12">Last Active</div>
+                            <div className="w-2/12">Last Start</div>


question: If creation timestamp is still important, could we add a tooltip when hovering on the last start timestamp with the creation timestamp (e.g. Created 2 days ago)?

Not sure it is important

OK, let's drop it for now.

gtsiolis · 2021-03-19T08:44:28Z

components/dashboard/src/components/Modal.tsx

                    {props.closeable !== false && (
-                        <div className="absolute right-9 top-8 cursor-pointer" onClick={props.onClose}>
+                        <div className="absolute right-9 top-8 cursor-pointer" onClick={close}>


issue: The close (X) button is missing a hover state. ⛵

Default Hover

gtsiolis · 2021-03-19T08:46:53Z

components/dashboard/src/components/Modal.tsx

                    {props.closeable !== false && (
-                        <div className="absolute right-9 top-8 cursor-pointer" onClick={props.onClose}>
+                        <div className="absolute right-9 top-8 cursor-pointer" onClick={close}>


thought: Regarding the icons, we could use them as SVGs or import the npm package for the icons library but it's not stable yet, see Usage. See icon assets in relevant discussion.

Agree, we should decide on how to deal with it.

Added follow-up in #3508.

gtsiolis · 2021-03-19T09:11:19Z

components/dashboard/src/workspaces/WorkspaceEntry.tsx

-                    <p>Do you really want to delete this workspace?</p>
+                <h3>Delete Workspace</h3>
+                <div className="border-t border-gray-200 mt-2 -mx-6 px-6 py-2">
+                    <p>Do you really want to delete the following workspace?</p>


suggestion: Maybe make this less aggressive. 😇

Suggested change

<p>Do you really want to delete the following workspace?</p>

<p class="mt-1 mb-2 text-base">Are you sure you want to delete this workspace?</p>

gtsiolis · 2021-03-19T09:19:25Z

components/dashboard/src/workspaces/WorkspaceEntry.tsx

+                <h3>Delete Workspace</h3>
+                <div className="border-t border-gray-200 mt-2 -mx-6 px-6 py-2">
+                    <p>Do you really want to delete the following workspace?</p>
+                    <div className="w-full p-3">


nitpick: Minor modal pixel lift. ⭐

Suggested change

<div className="w-full p-3">

<div className="w-full p-4 mb-2 bg-gray-100 rounded-xl group bg-gray-100">

gtsiolis · 2021-03-19T09:20:31Z

components/dashboard/src/workspaces/WorkspaceEntry.tsx

                </div>
-                <div className="flex">
-                    <div className="flex-1"></div>
+                <div className="flex justify-end mt-4">


nitpick: Minor modal pixel lift.

Suggested change

<div className="flex justify-end mt-4">

<div className="flex justify-end mt-5">

gtsiolis · 2021-03-19T09:21:43Z

components/dashboard/src/workspaces/WorkspaceEntry.tsx

-                <div className="py-4">
-                    <p>Do you really want to delete this workspace?</p>
+                <h3>Delete Workspace</h3>
+                <div className="border-t border-gray-200 mt-2 -mx-6 px-6 py-2">


nitpick: Modal overhaul.

Suggested change

<div className="border-t border-gray-200 mt-2 -mx-6 px-6 py-2">

<div className="border-t border-b border-gray-200 mt-2 -mx-6 px-6 py-2">

BEFORE AFTER

gtsiolis · 2021-03-19T09:22:06Z

components/dashboard/src/workspaces/WorkspaceEntry.tsx

                    <button className="cursor-pointer px-3 py-2 text-white text-sm rounded-md border-2 border-red-800 bg-red-600 hover:bg-red-800"
-                        onClick={()=>getGitpodService().server.deleteWorkspace(ws.id)}>
+                        onClick={() => model.deleteWorkspace(ws.id)}>
                        Delete


nitpick: Let's make this more actionable and descriptive.

Suggested change

Delete

Delete Workspace

the modal dialog should - close on ESC - close on click outside modal but not within - handle enter

svenefftinge · 2021-03-19T10:58:26Z

filed #3507

svenefftinge added 2 commits March 18, 2021 19:35

[dashboard] allow open workspace in new tab

f66483f

[dashboard] fix deletion of workspaces

a406854

svenefftinge requested review from AlexTugarev, gtsiolis and jankeromnes and removed request for AlexTugarev March 18, 2021 21:32

svenefftinge commented Mar 19, 2021

View reviewed changes

gtsiolis self-assigned this Mar 19, 2021

AlexTugarev reviewed Mar 19, 2021

View reviewed changes

gtsiolis approved these changes Mar 19, 2021

View reviewed changes

gtsiolis mentioned this pull request Mar 19, 2021

[dashboard] avatar menu #3491

Merged

[dashboard] improve modal

47aa569

the modal dialog should - close on ESC - close on click outside modal but not within - handle enter

svenefftinge force-pushed the se/misc-dashboard branch from 88d3764 to 47aa569 Compare March 19, 2021 10:56

svenefftinge mentioned this pull request Mar 19, 2021

[dashboard] Misc outstanding issues #3507

Closed

12 tasks

svenefftinge merged commit 4b92cdd into dashboard-v2 Mar 19, 2021

svenefftinge deleted the se/misc-dashboard branch March 19, 2021 10:58

gtsiolis mentioned this pull request Mar 19, 2021

Consider importing an icon library for icons #3508

Closed

	<p>Do you really want to delete the following workspace?</p>
	<p class="mt-1 mb-2 text-base">Are you sure you want to delete this workspace?</p>

	<div className="w-full p-3">
	<div className="w-full p-4 mb-2 bg-gray-100 rounded-xl group bg-gray-100">

	<div className="flex justify-end mt-4">
	<div className="flex justify-end mt-5">

	<div className="border-t border-gray-200 mt-2 -mx-6 px-6 py-2">
	<div className="border-t border-b border-gray-200 mt-2 -mx-6 px-6 py-2">

[dashboard] misc dashboard improvements #3496

[dashboard] misc dashboard improvements #3496

Uh oh!

Conversation

svenefftinge commented Mar 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

svenefftinge Mar 19, 2021 • 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!

gtsiolis commented Mar 19, 2021 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

gtsiolis left a comment

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!

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!

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!

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!

svenefftinge commented Mar 19, 2021

Uh oh!

Uh oh!

svenefftinge commented Mar 18, 2021 •

edited

Loading

svenefftinge Mar 19, 2021 •

edited

Loading

gtsiolis commented Mar 19, 2021 •

edited by werft-gitpod-dev-com bot

Loading