-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[server][dashboard] Improve 'New Workspace' modal #7715
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.
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
Changes from all commits
f0f06ff
075ea6e
7e95890
5b1a984
d18033f
fc2328a
0eac52f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,175 @@ | ||||||||||||||
/** | ||||||||||||||
* Copyright (c) 2022 Gitpod GmbH. All rights reserved. | ||||||||||||||
* Licensed under the GNU Affero General Public License (AGPL). | ||||||||||||||
* See License-AGPL.txt in the project root for license information. | ||||||||||||||
*/ | ||||||||||||||
|
||||||||||||||
import { User } from "@gitpod/gitpod-protocol"; | ||||||||||||||
import React, { useContext, useEffect, useState } from "react"; | ||||||||||||||
import { Link } from "react-router-dom"; | ||||||||||||||
import { getGitpodService } from "../service/service"; | ||||||||||||||
import { UserContext } from "../user-context"; | ||||||||||||||
|
||||||||||||||
type SearchResult = string; | ||||||||||||||
type SearchData = SearchResult[]; | ||||||||||||||
|
||||||||||||||
const LOCAL_STORAGE_KEY = 'open-in-gitpod-search-data'; | ||||||||||||||
const MAX_DISPLAYED_ITEMS = 15; | ||||||||||||||
|
||||||||||||||
export default function RepositoryFinder(props: { initialQuery?: string }) { | ||||||||||||||
const { user } = useContext(UserContext); | ||||||||||||||
const [searchQuery, setSearchQuery] = useState<string>(props.initialQuery || ''); | ||||||||||||||
const [searchResults, setSearchResults] = useState<SearchResult[]>([]); | ||||||||||||||
const [selectedSearchResult, setSelectedSearchResult] = useState<SearchResult | undefined>(); | ||||||||||||||
|
||||||||||||||
const onResults = (results: SearchResult[]) => { | ||||||||||||||
if (JSON.stringify(results) !== JSON.stringify(searchResults)) { | ||||||||||||||
setSearchResults(results); | ||||||||||||||
setSelectedSearchResult(results[0]); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
const search = async (query: string) => { | ||||||||||||||
setSearchQuery(query); | ||||||||||||||
await findResults(query, onResults); | ||||||||||||||
if (await refreshSearchData(query, user)) { | ||||||||||||||
// Re-run search if the underlying search data has changed | ||||||||||||||
await findResults(query, onResults); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Up/Down keyboard navigation between results | ||||||||||||||
const onKeyDown = (event: React.KeyboardEvent) => { | ||||||||||||||
if (!selectedSearchResult) { | ||||||||||||||
return; | ||||||||||||||
} | ||||||||||||||
const selectedIndex = searchResults.indexOf(selectedSearchResult); | ||||||||||||||
const select = (index: number) => { | ||||||||||||||
// Implement a true modulus in order to "wrap around" (e.g. `select(-1)` should select the last result) | ||||||||||||||
// Source: https://stackoverflow.com/a/4467559/3461173 | ||||||||||||||
const n = Math.min(searchResults.length, MAX_DISPLAYED_ITEMS); | ||||||||||||||
setSelectedSearchResult(searchResults[((index % n) + n) % n]); | ||||||||||||||
} | ||||||||||||||
if (event.key === 'ArrowDown') { | ||||||||||||||
event.preventDefault(); | ||||||||||||||
select(selectedIndex + 1); | ||||||||||||||
return; | ||||||||||||||
} | ||||||||||||||
if (event.key === 'ArrowUp') { | ||||||||||||||
event.preventDefault(); | ||||||||||||||
select(selectedIndex - 1); | ||||||||||||||
return; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
useEffect(() => { | ||||||||||||||
const element = document.querySelector(`a[href='/#${selectedSearchResult}']`); | ||||||||||||||
if (element) { | ||||||||||||||
element.scrollIntoView({ behavior: 'smooth', block: 'nearest' }); | ||||||||||||||
} | ||||||||||||||
}, [selectedSearchResult]); | ||||||||||||||
|
||||||||||||||
const onSubmit = (event: React.FormEvent) => { | ||||||||||||||
event.preventDefault(); | ||||||||||||||
if (selectedSearchResult) { | ||||||||||||||
window.location.href = '/#' + selectedSearchResult; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return <form onSubmit={onSubmit}> | ||||||||||||||
<div className="flex px-4 rounded-xl border border-gray-300 dark:border-gray-500"> | ||||||||||||||
<div className="py-4"> | ||||||||||||||
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 16 16" width="16" height="16"><path fill="#A8A29E" d="M6 2a4 4 0 100 8 4 4 0 000-8zM0 6a6 6 0 1110.89 3.477l4.817 4.816a1 1 0 01-1.414 1.414l-4.816-4.816A6 6 0 010 6z" /></svg> | ||||||||||||||
</div> | ||||||||||||||
<input type="search" className="flex-grow" placeholder="Search repositories and examples" autoFocus value={searchQuery} onChange={e => search(e.target.value)} onKeyDown={onKeyDown} /> | ||||||||||||||
</div> | ||||||||||||||
<div className="mt-3 -mx-5 px-5 flex flex-col space-y-2 h-64 overflow-y-auto"> | ||||||||||||||
{searchQuery === '' && searchResults.length === 0 && | ||||||||||||||
jankeromnes marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
<div className="mt-12 mx-auto w-96 text-gray-500"> | ||||||||||||||
Paste a <a className="gp-link" href="https://www.gitpod.io/docs/context-urls">repository context URL</a>, or start typing to see suggestions from: | ||||||||||||||
<ul className="list-disc mt-4 pl-7 flex flex-col space-y-1"> | ||||||||||||||
<li>Your recent repositories</li> | ||||||||||||||
<li>Your repositories from <Link className="gp-link" to="/integrations">connected integrations</Link></li> | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: The page redirect when clicking this feels a bit abrupt, no? 😬 suggestion: What if, we skipped linking to the integrations on this empty state and only prompted users to connect more providers when they couldn't find the repository they were looking for? We could also close the modal when someone clicked on the link to connect more providers. What do you think? Cc @jldec
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Hmm, good catch, but I think that's how we agreed to implement links (i.e. no more Thankfully, you can still I think linking to integrations is still useful, as it provides a valuable clue on where results come from / how to get more results. 💭 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Correct and agree! To clarify I was not suggesting to open this link in a new tab but either There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! Went with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Follow up issue can be found in #8088. |
||||||||||||||
<li>Example repositories</li> | ||||||||||||||
</ul> | ||||||||||||||
</div>} | ||||||||||||||
{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)}> | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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": There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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! 👀
thought: Keeping the scrollbar near the scrollbar area could help us 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]. 🌟
gtsiolis marked this conversation as resolved.
Show resolved
Hide resolved
jankeromnes marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
{searchQuery.length < 2 | ||||||||||||||
? <span>{result}</span> | ||||||||||||||
: result.split(searchQuery).map((segment, index) => <span> | ||||||||||||||
{index === 0 ? <></> : <strong>{searchQuery}</strong>} | ||||||||||||||
{segment} | ||||||||||||||
</span>)} | ||||||||||||||
</a> | ||||||||||||||
)} | ||||||||||||||
{searchResults.length > MAX_DISPLAYED_ITEMS && | ||||||||||||||
<span className="mt-3 px-4 py-2 text-sm text-gray-400 dark:text-gray-500">{searchResults.length - MAX_DISPLAYED_ITEMS} more result{(searchResults.length - MAX_DISPLAYED_ITEMS) === 1 ? '' : 's'} found</span>} | ||||||||||||||
</div> | ||||||||||||||
</form>; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
function loadSearchData(): SearchData { | ||||||||||||||
const string = localStorage.getItem(LOCAL_STORAGE_KEY); | ||||||||||||||
if (!string) { | ||||||||||||||
return []; | ||||||||||||||
} | ||||||||||||||
try { | ||||||||||||||
const data = JSON.parse(string); | ||||||||||||||
return data; | ||||||||||||||
} catch (error) { | ||||||||||||||
console.warn('Could not load search data from local storage', error); | ||||||||||||||
return []; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
function saveSearchData(searchData: SearchData): void { | ||||||||||||||
try { | ||||||||||||||
window.localStorage.setItem(LOCAL_STORAGE_KEY, JSON.stringify(searchData)); | ||||||||||||||
} catch (error) { | ||||||||||||||
console.warn('Could not save search data into local storage', error); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
let refreshSearchDataPromise: Promise<boolean> | undefined; | ||||||||||||||
export async function refreshSearchData(query: string, user: User | undefined): Promise<boolean> { | ||||||||||||||
if (refreshSearchDataPromise) { | ||||||||||||||
// Another refresh is already in progress, no need to run another one in parallel. | ||||||||||||||
return refreshSearchDataPromise; | ||||||||||||||
} | ||||||||||||||
refreshSearchDataPromise = actuallyRefreshSearchData(query, user); | ||||||||||||||
const didChange = await refreshSearchDataPromise; | ||||||||||||||
refreshSearchDataPromise = undefined; | ||||||||||||||
return didChange; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Fetch all possible search results and cache them into local storage | ||||||||||||||
async function actuallyRefreshSearchData(query: string, user: User | undefined): Promise<boolean> { | ||||||||||||||
console.log('refreshing search data'); | ||||||||||||||
const oldData = loadSearchData(); | ||||||||||||||
const newData = await getGitpodService().server.getSuggestedContextURLs(); | ||||||||||||||
if (JSON.stringify(oldData) !== JSON.stringify(newData)) { | ||||||||||||||
console.log('new data:', newData); | ||||||||||||||
saveSearchData(newData); | ||||||||||||||
return true; | ||||||||||||||
} | ||||||||||||||
return false; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
async function findResults(query: string, onResults: (results: string[]) => void) { | ||||||||||||||
if (!query) { | ||||||||||||||
onResults([]); | ||||||||||||||
return; | ||||||||||||||
} | ||||||||||||||
const searchData = loadSearchData(); | ||||||||||||||
try { | ||||||||||||||
// If the query is a URL, and it's not present in the proposed results, "artificially" add it here. | ||||||||||||||
new URL(query); | ||||||||||||||
if (!searchData.includes(query)) { | ||||||||||||||
searchData.push(query); | ||||||||||||||
} | ||||||||||||||
} catch { | ||||||||||||||
} | ||||||||||||||
// console.log('searching', query, 'in', searchData); | ||||||||||||||
onResults(searchData.filter(result => result.toLowerCase().includes(query.toLowerCase()))); | ||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/** | ||
* Copyright (c) 2022 Gitpod GmbH. All rights reserved. | ||
* Licensed under the GNU Affero General Public License (AGPL). | ||
* See License-AGPL.txt in the project root for license information. | ||
*/ | ||
|
||
import { useEffect, useState } from "react"; | ||
import RepositoryFinder from "../components/RepositoryFinder"; | ||
|
||
export default function Open() { | ||
const [ initialQuery, setInitialQuery ] = useState<string | undefined>(); | ||
|
||
// Support pre-filling the search bar via the URL hash | ||
useEffect(() => { | ||
const onHashChange = () => { | ||
const hash = window.location.hash.slice(1); | ||
if (hash) { | ||
setInitialQuery(hash); | ||
} | ||
} | ||
onHashChange(); | ||
window.addEventListener('hashchange', onHashChange, false); | ||
return () => { | ||
window.removeEventListener('hashchange', onHashChange, false); | ||
} | ||
}, []); | ||
|
||
return <div className="mt-24 mx-auto w-96 flex flex-col items-stretch"> | ||
<h1 className="text-center">Open in Gitpod</h1> | ||
<div className="mt-8"> | ||
<RepositoryFinder initialQuery={initialQuery}/> | ||
</div> | ||
</div>; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.