Skip to content

Commit

Permalink
Improve assets table display (#7500)
Browse files Browse the repository at this point in the history
- Closes #7463
- Makes table header sticky
- Clips table body so it does not overlap table header

Other changes:
- Clip table header row so it does not overlap extra columns selector
- Hide extra columns selector on local backend (PM backend)
- Focus search bar if the keypress will type regular text
- Change row height from 40px to 32px
- Add "Share" button when the editor is open
- Make entire area of backend selector (Cloud <-> Local) clickable (previously, the padding was not clickable)
- Remove the up-arrow icon to open a project. Projects are now opened by switching to the project tab using the tab selector on the top left (or by double clicking the row).
- Fix opening newly created folder (previously its entries were appended to the end, rather than under the folder)
- Indent background of "name" column (the first column)
- Minor code style changes
- Add background back to "change password" modal (oops)
- Hide "open" context menu entry and show "stop" entry, when a project is currently running
- ℹ️ It might be a good idea to support the "open" action on directories as well, however it is difficult without the assets table refactor in #7540. As such, this functionality will not be added in this PR.
- Fix horizontal padding on "sign in" user menu entry
- Hide email/password validation when using oauth logins

More fixes for assets list:
- Project is inserted at start of list when there are no existing projects
- Project is inserted at start of children when there are no existing children
- Deleting a folder collapses it (hides its descendants)
- Adding children to a newly created folder puts them at the correct depth, rather than depth 1

# Important Notes
None
  • Loading branch information
somebody1234 authored Aug 28, 2023
1 parent 8b3578b commit e0ef0a2
Show file tree
Hide file tree
Showing 49 changed files with 723 additions and 359 deletions.
2 changes: 1 addition & 1 deletion app/ide-desktop/debugGlobals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function assert(invariant: boolean, message: string, logger: Logger = con
}
}

// This is required to make the definition `Object.prototype.d` not error.
// This is required to make the definition `Object.prototype.$d$` not error.
// eslint-disable-next-line no-restricted-syntax
declare global {
// Documentation is already inherited.
Expand Down
6 changes: 0 additions & 6 deletions app/ide-desktop/lib/assets/arrow_up.svg

This file was deleted.

11 changes: 5 additions & 6 deletions app/ide-desktop/lib/assets/close.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
98 changes: 80 additions & 18 deletions app/ide-desktop/lib/common/src/detect.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,5 @@
/** @file Helper functions for environment detection. */

// ===========================
// === isRunningInElectron ===
// ===========================

/** Returns `true` if running in Electron, else `false`.
* This is used to determine whether to use a `MemoryRouter` (stores history in an array)
* or a `BrowserRouter` (stores history in the path of the URL).
* It is also used to determine whether to send custom state to Amplify for a workaround. */
export function isRunningInElectron() {
return /electron/i.test(navigator.userAgent)
}

// ================
// === Platform ===
// ================
Expand All @@ -27,11 +15,11 @@ export enum Platform {
/** Return the platform the app is currently running on.
* This is used to determine whether `metaKey` or `ctrlKey` is used in shortcuts. */
export function platform(): Platform {
if (/windows/i.test(navigator.userAgent)) {
if (isOnWindows()) {
return Platform.windows
} else if (/mac os/i.test(navigator.userAgent)) {
} else if (isOnMacOS()) {
return Platform.macOS
} else if (/linux/i.test(navigator.userAgent)) {
} else if (isOnLinux()) {
return Platform.linux
} else {
return Platform.unknown
Expand All @@ -40,20 +28,94 @@ export function platform(): Platform {

/** Return whether the device is running Windows. */
export function isOnWindows() {
return platform() === Platform.windows
return /windows/i.test(navigator.userAgent)
}

/** Return whether the device is running macOS. */
export function isOnMacOS() {
return platform() === Platform.macOS
return /mac os/i.test(navigator.userAgent)
}

/** Return whether the device is running Linux. */
export function isOnLinux() {
return platform() === Platform.linux
return /linux/i.test(navigator.userAgent)
}

/** Return whether the device is running an unknown OS. */
export function isOnUnknownOS() {
return platform() === Platform.unknown
}

// ===============
// === Browser ===
// ===============

/** Possible browsers that the app may run on. */
export enum Browser {
unknown = 'Unknown browser',
electron = 'Electron',
chrome = 'Chrome',
edge = 'Edge',
firefox = 'Firefox',
safari = 'Safari',
opera = 'Opera',
}

/** Return the platform the app is currently running on.
* This is used to determine whether `metaKey` or `ctrlKey` is used in shortcuts. */
export function browser(): Browser {
if (isOnElectron()) {
return Browser.electron
// This MUST be above Chrome as it is Chromium-based.
} else if (isOnEdge()) {
return Browser.opera
// This MUST be above Chrome as it is Chromium-based.
} else if (isOnOpera()) {
return Browser.edge
} else if (isOnChrome()) {
return Browser.chrome
} else if (isOnFirefox()) {
return Browser.firefox
} else if (isOnSafari()) {
return Browser.safari
} else {
return Browser.unknown
}
}
/** Returns `true` if running in Electron, else `false`.
* This is used to determine whether to use a `MemoryRouter` (stores history in an array)
* or a `BrowserRouter` (stores history in the path of the URL).
* It is also used to determine whether to send custom state to Amplify for a workaround. */
export function isOnElectron() {
return /electron/i.test(navigator.userAgent)
}

/** Return whether the current browser is Microsoft Edge. */
export function isOnEdge() {
return /edg/i.test(navigator.userAgent)
}

/** Return whether the current browser is Opera. */
export function isOnOpera() {
return /opr/i.test(navigator.userAgent)
}

/** Return whether the current browser is Google Chrome. */
export function isOnChrome() {
return /chrome/i.test(navigator.userAgent)
}

/** Return whether the current browser is Mozilla Firefox. */
export function isOnFirefox() {
return /firefox/i.test(navigator.userAgent)
}

/** Return whether the current browser is Safari. */
export function isOnSafari() {
return /safari/i.test(navigator.userAgent)
}

/** Return whether the current browser is not a recognized browser. */
export function isOnUnknownBrowser() {
return browser() === Browser.unknown
}
3 changes: 1 addition & 2 deletions app/ide-desktop/lib/content/esbuild-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ export function bundlerOptions(args: Arguments) {
'.css': 'copy',
'.map': 'copy',
'.wasm': 'copy',
// The `file` loader copies the file, and replaces the import with the path to the file.
'.svg': 'file',
'.svg': 'dataurl',
'.png': 'file',
'.ttf': 'copy',
},
Expand Down
2 changes: 1 addition & 1 deletion app/ide-desktop/lib/content/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const FETCH_TIMEOUT = 300
// === Live reload ===
// ===================

if (IS_DEV_MODE && !detect.isRunningInElectron()) {
if (IS_DEV_MODE && !detect.isOnElectron()) {
new EventSource(ESBUILD_PATH).addEventListener(ESBUILD_EVENT_NAME, () => {
// This acts like `location.reload`, but it preserves the query-string.
// The `toString()` is to bypass a lint without using a comment.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ export class Cognito {
*
* See: https://github.com/aws-amplify/amplify-js/issues/3391#issuecomment-756473970 */
private customState() {
return detect.isRunningInElectron() ? window.location.pathname : null
return detect.isOnElectron() ? window.location.pathname : null
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,20 @@ export interface InputProps extends React.InputHTMLAttributes<HTMLInputElement>
error?: string
validate?: boolean
setValue: (value: string) => void
shouldReportValidityRef?: React.MutableRefObject<boolean>
}

/** A component for authentication from inputs, with preset styles. */
export default function Input(props: InputProps) {
const { setValue, error, validate = false, onChange, onBlur, ...passThrough } = props
const {
setValue,
error,
validate = false,
shouldReportValidityRef,
onChange,
onBlur,
...passThrough
} = props
const [reportTimeoutHandle, setReportTimeoutHandle] = React.useState<number | null>(null)
const [hasReportedValidity, setHasReportedValidity] = React.useState(false)
const [wasJustBlurred, setWasJustBlurred] = React.useState(false)
Expand All @@ -39,17 +48,27 @@ export default function Input(props: InputProps) {
}
const currentTarget = event.currentTarget
if (error != null) {
currentTarget.setCustomValidity('')
currentTarget.setCustomValidity(currentTarget.checkValidity() ? '' : error)
currentTarget.setCustomValidity(
currentTarget.checkValidity() ||
shouldReportValidityRef?.current === false
? ''
: error
)
}
if (hasReportedValidity) {
if (currentTarget.checkValidity()) {
if (
shouldReportValidityRef?.current === false ||
currentTarget.checkValidity()
) {
setHasReportedValidity(false)
}
} else {
setReportTimeoutHandle(
window.setTimeout(() => {
if (!currentTarget.reportValidity()) {
if (
shouldReportValidityRef?.current !== false &&
!currentTarget.reportValidity()
) {
setHasReportedValidity(true)
}
}, DEBOUNCE_MS)
Expand All @@ -65,9 +84,11 @@ export default function Input(props: InputProps) {
setHasReportedValidity(false)
} else {
const currentTarget = event.currentTarget
setTimeout(() => {
currentTarget.reportValidity()
}, 0)
if (shouldReportValidityRef?.current !== false) {
if (!currentTarget.reportValidity()) {
event.preventDefault()
}
}
setWasJustBlurred(true)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export default function Login() {
const [email, setEmail] = React.useState(initialEmail ?? '')
const [password, setPassword] = React.useState('')
const [isSubmitting, setIsSubmitting] = React.useState(false)
const shouldReportValidityRef = React.useRef(true)

return (
<div className="min-h-screen flex flex-col items-center justify-center">
Expand All @@ -53,6 +54,9 @@ export default function Login() {
Login To Your Account
</div>
<button
onMouseDown={() => {
shouldReportValidityRef.current = false
}}
onClick={async event => {
event.preventDefault()
await signInWithGoogle()
Expand All @@ -63,6 +67,9 @@ export default function Login() {
<span>Sign Up or Login with Google</span>
</button>
<button
onMouseDown={() => {
shouldReportValidityRef.current = false
}}
onClick={async event => {
event.preventDefault()
await signInWithGitHub()
Expand All @@ -85,6 +92,7 @@ export default function Login() {
event.preventDefault()
setIsSubmitting(true)
await signInWithPassword(email, password)
shouldReportValidityRef.current = true
setIsSubmitting(false)
}}
>
Expand All @@ -108,6 +116,7 @@ export default function Login() {
placeholder="E-Mail Address"
value={email}
setValue={setEmail}
shouldReportValidityRef={shouldReportValidityRef}
/>
</div>
</div>
Expand All @@ -133,6 +142,7 @@ export default function Login() {
error={validation.PASSWORD_ERROR}
value={password}
setValue={setPassword}
shouldReportValidityRef={shouldReportValidityRef}
/>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,9 @@ export function AuthProvider(props: AuthProviderProps) {
// This prevents a busy loop when request blocking is enabled in DevTools.
// The UI will be blank indefinitely. This is intentional, since for real
// network outages, `navigator.onLine` will be false.
await new Promise(resolve => setTimeout(resolve, REQUEST_DELAY_MS))
await new Promise<void>(resolve => {
window.setTimeout(resolve, REQUEST_DELAY_MS)
})
}
}
const url = new URL(location.href)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export interface AppProps {
export default function App(props: AppProps) {
// This is a React component even though it does not contain JSX.
// eslint-disable-next-line no-restricted-syntax
const Router = detect.isRunningInElectron() ? router.MemoryRouter : router.BrowserRouter
const Router = detect.isOnElectron() ? router.MemoryRouter : router.BrowserRouter
/** Note that the `Router` must be the parent of the `AuthProvider`, because the `AuthProvider`
* will redirect the user between the login/register pages and the dashboard. */
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,3 @@ export function spliceAfter<T>(array: T[], items: T[], predicate: (value: T) =>
export function splicedAfter<T>(array: T[], items: T[], predicate: (value: T) => boolean) {
return spliceAfter(Array.from(array), items, predicate)
}

/** Insert items to replace the first item for which the `predicate` returns `true`.
* Do not insert items if the `predicate` never returns `true`. */
export function spliceReplacing<T>(array: T[], items: T[], predicate: (value: T) => boolean) {
const index = array.findIndex(predicate)
if (index !== NOT_FOUND) {
array.splice(index, 1, ...items)
}
return array
}

/** Return a copy of the array, with items replacing the first item for which `predicate` is `true`.
* The items are not inserted if the `predicate` never returns `true`. */
export function splicedReplacing<T>(array: T[], items: T[], predicate: (value: T) => boolean) {
return spliceReplacing(Array.from(array), items, predicate)
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/** @file Type definitions common between all backends. */
import * as React from 'react'

import * as dateTime from './dateTime'
import * as newtype from '../newtype'
Expand Down Expand Up @@ -159,7 +160,7 @@ export interface ProjectStateType {
/* eslint-enable @typescript-eslint/naming-convention */
}

export const IS_PROJECT_STATE_OPENING_OR_OPENED: Record<ProjectState, boolean> = {
export const DOES_PROJECT_STATE_INDICATE_VM_EXISTS: Record<ProjectState, boolean> = {
[ProjectState.created]: false,
[ProjectState.new]: false,
[ProjectState.openInProgress]: true,
Expand Down Expand Up @@ -221,6 +222,8 @@ export interface Project extends ListedProject {
export interface ProjectStartupInfo {
project: Project
projectAsset: ProjectAsset
// This MUST BE optional because it is lost when `JSON.stringify`ing to put in `localStorage`.
setProjectAsset?: React.Dispatch<React.SetStateAction<ProjectAsset>>
backendType: BackendType
accessToken: string | null
}
Expand Down
Loading

0 comments on commit e0ef0a2

Please sign in to comment.