[server][dashboard] Improve 'New Workspace' modal #7715

jankeromnes · 2022-01-20T11:01:15Z

Description

Improve the 'New Workspace' modal with a search input, keyboard navigation, and a new context URL suggestion API.

Related Issue(s)

Fixes #7564

How to test

Click on New Workspace (or hit Ctrl/Cmd + O from anywhere in the dashboard)
Type to find a suggested repository* or paste any context URL and hit Enter

*currently sources suggestions from:

Example repos
Your recent repos
Your repos from all your connected integrations

Release Notes

[server][dashboard] Improve 'New Workspace' modal with a search input, keyboard navigation, and a new context URL suggestion API

Documentation

/uncc

jankeromnes · 2022-01-21T14:13:19Z

components/dashboard/src/components/Modal.tsx

@@ -51,7 +51,7 @@ export default function Modal(props: {
    return (
        <div className="fixed top-0 left-0 bg-black bg-opacity-70 z-50 w-screen h-screen">
            <div className="w-screen h-screen align-middle" style={{display: 'table-cell'}}>
-                <div className={"relative bg-white dark:bg-gray-900 border border-gray-200 dark:border-gray-800 rounded-xl p-6 max-w-lg mx-auto text-left text-gray-600 " + (props.className || '')} onClick={e => e.stopPropagation()}>
+                <div className={"relative bg-white dark:bg-gray-900 border border-gray-200 dark:border-gray-800 rounded-xl p-6 max-w-lg mx-auto text-left " + (props.className || '')} onClick={e => e.stopPropagation()}>


This was making all text in all modals text-gray-600 by default, which was impractical.

I've removed it and checked several Modals, but I hope I haven't missed any side effects of this fix.

components/dashboard/src/components/RepositoryFinder.tsx

jankeromnes · 2022-01-21T15:26:17Z

Alright, I think the current state is pretty cool. ✨

Initial state	Type anything	Paste

@gtsiolis @jldec Could you please take a look? 🙏

Cool things to try:

Up/Down arrow, Enter
Cmd+~~K~~ shortcut (or ~~P~~, O, or with Ctrl)
type rust, next, petclinic
paste any context URL
connect GitLab and Bitbucket in /integrations then try to find your repos in the Modal

gtsiolis · 2022-01-21T15:59:52Z

Looking at this now! 👀

gtsiolis

Looks great, @jankeromnes! 🔮

Added some first round review comments below with some non-blocking questions. 🥊

Approving to unblock merging but holding in case we could tackle any of the comments below and since someone else could take a closer look at the code changes.

/hold

components/dashboard/src/workspaces/Workspaces.tsx

components/dashboard/src/workspaces/StartWorkspaceModal.tsx

components/dashboard/src/workspaces/Workspaces.tsx

components/dashboard/src/components/RepositoryFinder.tsx

gtsiolis · 2022-01-21T16:18:27Z

components/dashboard/src/components/RepositoryFinder.tsx

+        </div>
+        <div className="mt-3 flex flex-col space-y-2 h-64 overflow-y-auto" id="search-results">
+            {searchResults.slice(0, MAX_DISPLAYED_ITEMS).map((result, index) =>
+                <a className={`px-4 py-3 rounded-xl` + (result === selectedSearchResult ? ' bg-gray-600 text-gray-50 dark:bg-gray-700' : '')} href={`/#${result}`} key={`search-result-${index}`} onMouseEnter={() => setSelectedSearchResult(result)}>


suggestion: It would be probably good to add some right margin here to avoid squashing the highlighted reasult entry with the visible scrollbar. What do you think?

Suggested change

<a className={`px-4 py-3 rounded-xl` + (result === selectedSearchResult ? ' bg-gray-600 text-gray-50 dark:bg-gray-700' : '')} href={`/#${result}`} key={`search-result-${index}`} onMouseEnter={() => setSelectedSearchResult(result)}>

<a className={`px-4 py-3 rounded-xl mr-2` + (result === selectedSearchResult ? ' bg-gray-600 text-gray-50 dark:bg-gray-700' : '')} href={`/#${result}`} key={`search-result-${index}`} onMouseEnter={() => setSelectedSearchResult(result)}>

BEFORE AFTER

Uh oh 😬 I'm not a big fan of either, and I think I'd prefer having the results be the same width as the input. I'll see if I can improve this somehow.

Yeah, no biggie. Regarding width, same width would be nice but that could require us to move the scrollbar outside the scrolling area. Posting below a slightly related design with scrollbars from #4948 to better visualize the problem. In any case, feel free to leave this as is, too. 🏓

@gtsiolis Okay, I believe I've found an elegant fix for this 😁 I've simply made the scroll zone wider (negative X-margin) and compensated with some padding (positive X-padding). This doesn't require changing the size of other elements, and has the effect of "simply moving the scrollbar a bit to the right":

Thanks for giving this a go, @jankeromnes!

suggestion: I think going with the simple inner right padding for now sounds better.

issue: Moving the scrollbar outside could make the user feel like they can scroll the through the whole modal inner elements, including title, text input, etc, not just the repository list. This also becomes more visible or an issue when always showing scrollbars, see screenshots below. This is also what I see! 👀

Light Dark

thought: Keeping the scrollbar near the scrollbar area could help us 🅰️ use better layouts like the one with the gray background mentioned in #7715 (comment), 🅱️ potentially introduce more fixed elements like a bottom toolbar for surfacing results found, etc.

Regarding the scrollbar blindness issue we discussed a few days ago, here's a relevant thread[1] but unfortunately the original article has been removed. I could only find this partial screenshot[2]. 🌟

components/dashboard/src/components/RepositoryFinder.tsx

components/dashboard/src/workspaces/StartWorkspaceModal.tsx

roboquat · 2022-01-21T16:29:46Z

LGTM label has been added.

Git tree hash: c63d3e13f3b4e80b6d013910e2fbd1b5d4ff4265

svenefftinge · 2022-01-24T17:38:22Z

Would be great if it would propose the repositories I care about.
This graphql query would return the first 100 repos I have contributed to. It's actually pretty fast query.

query { 
  user(login: "svenefftinge") {
      repositoriesContributedTo(includeUserRepositories: true, first: 100) {
        edges {
          node {
            nameWithOwner
            shortDescriptionHTML(limit: 120)
            url
          }
        }
      }
    }
}

Try here.

jankeromnes · 2022-01-24T19:58:09Z

Exciting find, many thanks @svenefftinge!

Actually this PR should already fetch all your GitHub repositories (either just svenefftinge/* or also from orgs, I'm not sure). It uses the same code that feeds the "New Project" wizard options (just with a faster UX), which unfortunately doesn't work in this preview because I haven't connected a GitHub App yet.

You can test it by connecting GitLab and Bitbucket in /integrations though.

Will add a GitHub App in the next deployment.

However, I'll likely keep your query too, because it can probably find even more relevant results than what we already have. Thanks again!

svenefftinge

Cool! I belive it would be better if suggestions are displayed already when you first open the dialog with an empty input. WDYT?

components/server/src/workspace/gitpod-server-impl.ts

jankeromnes · 2022-01-24T20:34:54Z

Cool! I belive it would be better if suggestions are displayed already when you first open the dialog with an empty input. WDYT?

I also like this idea, and you can sort of simulate it by typing then removing a letter. (The order of the suggestions is currently alphabetical, which we could probably make more relevant.)

We discussed this earlier today in our team meeting. Another option could be to have a helpful empty state, which explains what exactly is happening here / what you should do / where the results come from. But if we can immediately show a list of the most relevant suggestions, maybe we don't even need to "explain" the feature with an empty state (i.e. you just click on New Workspace, then you immediately see the repo you're interested in, and voilà, job done, no "learning" was required because the feature was intuitive).

svenefftinge · 2022-01-24T20:38:00Z

components/server/src/workspace/gitpod-server-impl.ts

+            if (!provider) {
+                return;
+            }
+            promises.push(this.getProviderRepositoriesForUser(ctx, { provider }).then(userRepos => {


I don't think reusing this method is a good idea as it limits the list of repos to the repos that have the app installed. Also fetches projects etc. IMHO this should be entirely about proposing great matches for a user, no matter an app is installed. We will grow out of what this method does pretty soon anyway.

Good point, thanks. Added a TODO comment as a first start.

roboquat · 2022-02-04T18:49:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gtsiolis, jldec

Associated issue: #7564

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~components/dashboard/OWNERS~~ [gtsiolis,jldec]
~~components/gitpod-protocol/OWNERS~~ [gtsiolis,jldec]
~~components/server/OWNERS~~ [gtsiolis,jldec]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gtsiolis

Thanks again @jankeromnes for pushing this through the finish line! 🏁

Cross-linking below one last round of comments related to UX that we could improve before merging this. Up for discussing more about these or leaving any of these for the next iteration and out of the scope of this PR if needed. Cc @jldec

gtsiolis

Left two more minor comments regarding the keyboard shortcut inside the button label.

components/dashboard/src/workspaces/start-workspace-modal-context.tsx

jankeromnes · 2022-02-08T11:30:12Z

Cross-linking below one last round of comments related to UX that we could improve before merging this.

Many thanks @gtsiolis for summarizing remaining concerns! Will address them or open follow-up issues.

Empty state placeholder, text input design, modal background, and removal of the background page load.

Removed the background page load (🅰️ -- good catch!) and opened #8088 for the remaining design items.

Results found text design.

✅

Restoring empty state.

✅

Text input placeholder.

✅

Left two more minor comments regarding the keyboard shortcut inside the button label.

~~⏳ TODO(janx)~~ EDIT: ✅ (see comment above about OS consistency)

…, keyboard navigation, and a new context URL suggestion API

…Cmd/Ctrl+O)

jankeromnes · 2022-02-08T13:41:02Z

/unhold

jankeromnes · 2022-02-08T13:46:25Z

Okay, I think we've iterated enough here. 😅 Happy to iterate further with follow-up issues. 🚀

Could a kind team mate from @gitpod-io/engineering-webapp please approve this Pull Request again? 🙏 (lgtm label is still there, but new process...)

JanKoehnlein

Let's ship this!

jankeromnes added the approved label Jan 20, 2022

roboquat added do-not-merge/work-in-progress release-note team: webapp Issue belongs to the WebApp team labels Jan 20, 2022

This comment has been minimized.

Sign in to view

roboquat added the size/L label Jan 20, 2022

jankeromnes force-pushed the jx/open-in-gitpod branch from 282795a to f45eef3 Compare January 20, 2022 15:56

roboquat added size/XL and removed size/L labels Jan 20, 2022

jankeromnes force-pushed the jx/open-in-gitpod branch 3 times, most recently from 6c07882 to 0651cb2 Compare January 21, 2022 14:07

jankeromnes marked this pull request as ready for review January 21, 2022 14:12

roboquat removed the do-not-merge/work-in-progress label Jan 21, 2022

jankeromnes commented Jan 21, 2022

View reviewed changes

components/dashboard/src/components/RepositoryFinder.tsx Outdated Show resolved Hide resolved

jankeromnes force-pushed the jx/open-in-gitpod branch from 0651cb2 to a0d9bb8 Compare January 21, 2022 15:09

gtsiolis previously approved these changes Jan 21, 2022

View reviewed changes

roboquat added the do-not-merge/hold label Jan 21, 2022

roboquat assigned gtsiolis Jan 21, 2022

roboquat added the lgtm label Jan 21, 2022

svenefftinge reviewed Jan 24, 2022

View reviewed changes

components/server/src/workspace/gitpod-server-impl.ts Outdated Show resolved Hide resolved

svenefftinge reviewed Jan 24, 2022

View reviewed changes

jankeromnes force-pushed the jx/open-in-gitpod branch from a0d9bb8 to ce6be68 Compare January 25, 2022 09:03

roboquat dismissed gtsiolis’s stale review via 4f8645a February 7, 2022 11:07

gtsiolis reviewed Feb 7, 2022

View reviewed changes

jankeromnes dismissed jldec’s stale review via d727b94 February 7, 2022 16:38

jankeromnes force-pushed the jx/open-in-gitpod branch 2 times, most recently from d727b94 to 01ef76b Compare February 7, 2022 18:06

gtsiolis reviewed Feb 8, 2022

View reviewed changes

components/dashboard/src/workspaces/start-workspace-modal-context.tsx Outdated Show resolved Hide resolved

components/dashboard/src/workspaces/start-workspace-modal-context.tsx Outdated Show resolved Hide resolved

jankeromnes mentioned this pull request Feb 8, 2022

Design improvements for the new "New Workspace" modal #8088

Closed

jankeromnes force-pushed the jx/open-in-gitpod branch from 01ef76b to 80e4025 Compare February 8, 2022 11:29

jankeromnes added 7 commits February 8, 2022 13:35

[server][dashboard] Improve 'New Workspace' modal with a search input…

f0f06ff

…, keyboard navigation, and a new context URL suggestion API

[dashboard] Open 'New Workspace' Modal on {ctrl,cmd} + {k,o,p}

075ea6e

[dashboard] Enable the 'New Workspace' modal on all dashboard pages (…

7e95890

…Cmd/Ctrl+O)

[server] Log response status when GitHub Education API calls fail

5b1a984

[server] Get repository suggestions from all auth providers

d18033f

[server] Implement a RepositoryProvider.getUserRepos() method

fc2328a

[dashboard] Update 'Open in Gitpod' modal design to spec

0eac52f

jankeromnes force-pushed the jx/open-in-gitpod branch from 80e4025 to 0eac52f Compare February 8, 2022 13:35

roboquat removed the do-not-merge/hold label Feb 8, 2022

jankeromnes requested a review from a team February 8, 2022 13:46

JanKoehnlein approved these changes Feb 8, 2022

View reviewed changes

roboquat merged commit ac0afcf into main Feb 8, 2022

roboquat deleted the jx/open-in-gitpod branch February 8, 2022 13:48

jankeromnes mentioned this pull request Feb 16, 2022

Track keybinding usage/exploration in oder to make the dashboard more intuitive #8251

Closed

filiptronicek added a commit that referenced this pull request Mar 19, 2022

Remove console statements introduced in #7715

31afba6

filiptronicek added a commit that referenced this pull request Mar 19, 2022

Remove console statements introduced in #7715

672ee74

filiptronicek mentioned this pull request Mar 19, 2022

Remove console statements introduced in #7715 #8899

Merged

roboquat pushed a commit that referenced this pull request Mar 21, 2022

Remove console statements introduced in #7715

4fa1859

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[server][dashboard] Improve 'New Workspace' modal #7715

[server][dashboard] Improve 'New Workspace' modal #7715

jankeromnes commented Jan 20, 2022 •

edited

Loading

This comment has been minimized.

jankeromnes Jan 21, 2022 •

edited

Loading

jankeromnes commented Jan 21, 2022 •

edited

Loading

gtsiolis commented Jan 21, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment

gtsiolis Jan 21, 2022

jankeromnes Jan 24, 2022

gtsiolis Jan 24, 2022

jankeromnes Feb 1, 2022

gtsiolis Feb 2, 2022

roboquat commented Jan 21, 2022

svenefftinge commented Jan 24, 2022 •

edited

Loading

jankeromnes commented Jan 24, 2022 •

edited

Loading

svenefftinge left a comment

jankeromnes commented Jan 24, 2022 •

edited by werft-gitpod-dev-com bot

Loading

svenefftinge Jan 24, 2022

jankeromnes Jan 28, 2022

roboquat commented Feb 4, 2022

gtsiolis left a comment •

edited

Loading

gtsiolis left a comment

jankeromnes commented Feb 8, 2022 •

edited

Loading

jankeromnes commented Feb 8, 2022

jankeromnes commented Feb 8, 2022 •

edited

Loading

JanKoehnlein left a comment

	<a className={`px-4 py-3 rounded-xl` + (result === selectedSearchResult ? ' bg-gray-600 text-gray-50 dark:bg-gray-700' : '')} href={`/#${result}`} key={`search-result-${index}`} onMouseEnter={() => setSelectedSearchResult(result)}>
	<a className={`px-4 py-3 rounded-xl mr-2` + (result === selectedSearchResult ? ' bg-gray-600 text-gray-50 dark:bg-gray-700' : '')} href={`/#${result}`} key={`search-result-${index}`} onMouseEnter={() => setSelectedSearchResult(result)}>

[server][dashboard] Improve 'New Workspace' modal #7715

[server][dashboard] Improve 'New Workspace' modal #7715

Conversation

jankeromnes commented Jan 20, 2022 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

Documentation

This comment has been minimized.

jankeromnes Jan 21, 2022 • edited Loading

Choose a reason for hiding this comment

jankeromnes commented Jan 21, 2022 • edited Loading

gtsiolis commented Jan 21, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis left a comment

Choose a reason for hiding this comment

gtsiolis Jan 21, 2022

Choose a reason for hiding this comment

jankeromnes Jan 24, 2022

Choose a reason for hiding this comment

gtsiolis Jan 24, 2022

Choose a reason for hiding this comment

jankeromnes Feb 1, 2022

Choose a reason for hiding this comment

gtsiolis Feb 2, 2022

Choose a reason for hiding this comment

roboquat commented Jan 21, 2022

svenefftinge commented Jan 24, 2022 • edited Loading

jankeromnes commented Jan 24, 2022 • edited Loading

svenefftinge left a comment

Choose a reason for hiding this comment

jankeromnes commented Jan 24, 2022 • edited by werft-gitpod-dev-com bot Loading

svenefftinge Jan 24, 2022

Choose a reason for hiding this comment

jankeromnes Jan 28, 2022

Choose a reason for hiding this comment

roboquat commented Feb 4, 2022

gtsiolis left a comment • edited Loading

Choose a reason for hiding this comment

gtsiolis left a comment

Choose a reason for hiding this comment

jankeromnes commented Feb 8, 2022 • edited Loading

jankeromnes commented Feb 8, 2022

jankeromnes commented Feb 8, 2022 • edited Loading

JanKoehnlein left a comment

Choose a reason for hiding this comment

jankeromnes commented Jan 20, 2022 •

edited

Loading

jankeromnes Jan 21, 2022 •

edited

Loading

jankeromnes commented Jan 21, 2022 •

edited

Loading

gtsiolis commented Jan 21, 2022 •

edited by werft-gitpod-dev-com bot

Loading

svenefftinge commented Jan 24, 2022 •

edited

Loading

jankeromnes commented Jan 24, 2022 •

edited

Loading

jankeromnes commented Jan 24, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment •

edited

Loading

jankeromnes commented Feb 8, 2022 •

edited

Loading

jankeromnes commented Feb 8, 2022 •

edited

Loading