Skip to content
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

Fix Drive table #12397

Open
wants to merge 29 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
0d26d21
[dashboard/ProjectExecutionsCalendar] Fix missing `key`
somebody1234 Feb 28, 2025
f8043d0
[dashboard] Replace `gap-icon-with-text` with `gap-2`
somebody1234 Feb 28, 2025
b5ee64e
[dashboard/AssetsTable] Add background to "hidden columns" tray
somebody1234 Feb 28, 2025
951807d
Minor formatting change
somebody1234 Mar 3, 2025
b3dabf4
Fix sort arrow in "name" and "modified" being on a separate name
somebody1234 Mar 3, 2025
673b887
Format `Button/types.ts`
somebody1234 Mar 3, 2025
dc6ed86
[dashboard/Button] Replace `import *` with `import {}`
somebody1234 Mar 3, 2025
4938880
[dashboard/Breadcrumbs] Format
somebody1234 Mar 3, 2025
532930e
[dashboard/Drive] Infinite `staleTime` for root directory and parent …
somebody1234 Mar 3, 2025
c1b7331
[dashboard/Drive] Remove incorrect `staleTime` from "list directories…
somebody1234 Mar 3, 2025
5ec659a
[dashboard] Replace column sort icons with `Icon`s
somebody1234 Mar 3, 2025
65188bf
[dashboard/Button] Remove unnecessary `weight` prop from `Button`
somebody1234 Mar 3, 2025
81209bb
[dashboard/DatePicker] Fix reset button placement
somebody1234 Mar 3, 2025
e7c04af
[dashboard/DatePicker] Fix foreign placeholders (visible in "Activity…
somebody1234 Mar 3, 2025
85a9bb9
[dashboard] Fix margin to the left of short icons
somebody1234 Mar 3, 2025
e3ca1e7
[dashboard] Allow destructive code actions for "organize imports" Pre…
somebody1234 Mar 3, 2025
5e59056
[dashboard] Prettier + refactor imports
somebody1234 Mar 3, 2025
7ae9d02
[dashboard] Remove `<Text />` wrapper for button content
somebody1234 Mar 3, 2025
cfd6a5e
[dashboard/DatePicker] Fix segments always showing as placeholders
somebody1234 Mar 3, 2025
c6e79bd
[dashboard/ProjectExecution] Add truncated text and hover tooltip back
somebody1234 Mar 3, 2025
8f50c31
[dashboard/ProjectExecutionsCalendar] Avoid fading out date numbers i…
somebody1234 Mar 3, 2025
ef51cb2
[project-view] Prettier
somebody1234 Mar 3, 2025
a95b150
[dashboard/ProjectExecution] Show date instead of "Does not repeat" i…
somebody1234 Mar 3, 2025
f3f1aa6
Merge branch 'develop' into wip/sb/flat-assets-fixes
somebody1234 Mar 4, 2025
438c4f3
[dashboard/IconDisplay] Add and use component
somebody1234 Mar 5, 2025
2c3b1f2
Merge branch 'develop' into wip/sb/flat-assets-fixes
somebody1234 Mar 5, 2025
cab3917
[dashboard/IconDisplay] Fix tooltips
somebody1234 Mar 5, 2025
b40c8c9
[dashboard/IconDisplay] Add stories
somebody1234 Mar 5, 2025
e68c8b1
[dashboard/IconDisplay] Fix stories
somebody1234 Mar 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ const ButtonContent = memo(function ButtonContent(props: ButtonContentProps) {
styles={styles}
hideLoader={hideLoader}
/>
<Text color="inherit" truncate="1" className={styles.text()}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used to have a span with className={styles.text()}, haven't we?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, i tried extracting the css directly and it doesn't seem to work.
try running this commit in dev (staging env) and create a project execution (no repeats to minimize load, but long timezone to (try to) trigger the truncate class):

438c4f3

<Text weight="custom" color="inherit" truncate="1" className={styles.text()}>
Copy link
Contributor

@MrFlashAccount MrFlashAccount Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the second though - why did we add the <Text /> component here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right. mainly so I could get ellipsis + hover tooltip when the timezone is too long:
image

... granted, that should not be a button since it is not interactable, I am just using it as a hack to have an icon and text that are (or are supposed to be) pre-aligned

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any better suggestions?

Copy link
Contributor

@MrFlashAccount MrFlashAccount Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buttons (actions) shouldn't be truncated and should use active verbs. What does this button do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhh... nothing 😅 - as above, just using it as a hack to get a pre-aligned icon + text combo. i guess ideally we could refactor this out to a style-only component and then use that instead of a full on Button...

either way Text has been refactored away from Button again anyway

Copy link
Contributor

@MrFlashAccount MrFlashAccount Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, lets use just a <Text /> element instead of button here.

i guess ideally we could refactor this out to a style-only component

Yeah, we can add this pattern as a pattern (https://panda-css.com/docs/concepts/patterns) using tv function.

Also we do the same here so we can base the receipt on this: https://github.com/enso-org/enso/blob/wip/sergeigarin/flat-folder-structure-follow-up/app/gui/src/dashboard/components/Breadcrumbs/BreadcrumbItem.tsx#L206

{children}
</Text>
{hasAddon(addonEnd) && <div className={styles.addonEnd()}>{addonEnd}</div>}
Expand Down
55 changes: 22 additions & 33 deletions app/gui/src/dashboard/components/AriaComponents/Button/types.ts
Original file line number Diff line number Diff line change
@@ -1,53 +1,46 @@
/**
* @file
*
* Reusable types for the Button component
*/
import type * as aria from '#/components/aria'
/** @file Types for a `Button`. */
import type {
ButtonProps as AriaButtonProps,
ButtonRenderProps as AriaButtonRenderProps,
LinkRenderProps as AriaLinkRenderProps,
LinkProps,
Placement,
PressEvent,
} from '#/components/aria'
import type { ExtractFunction } from '#/utilities/tailwindVariants'
import type { ReactElement, ReactNode } from 'react'
import type { Addon, IconProp, TestIdProps } from '../types'
import type { BUTTON_STYLES, ButtonVariants } from './variants'

/**
* Position of a joined button
*/
/** Position of a joined button. */
export type PrivateJoinedButtonPosition = ButtonVariants['position']

/**
* Whether the button is joined
*/
/** Whether the button is joined. */
export type PrivateJoinedButton = ButtonVariants['isJoined']

/**
* Props for a joined button unlike other button props,
*/
/** Props for a joined button unlike other button props. */
export interface PrivateJoinedButtonProps {
readonly position: PrivateJoinedButtonPosition
readonly isJoined: NonNullable<PrivateJoinedButton>
}

/**
* Render props for a button.
*/
export interface ButtonRenderProps extends aria.ButtonRenderProps {
/** Render props for a button. */
export interface ButtonRenderProps extends AriaButtonRenderProps {
readonly isLoading: boolean
}

/**
* Render props for a link.
*/
export interface LinkRenderProps extends aria.LinkRenderProps {
/** Render props for a link. */
export interface LinkRenderProps extends AriaLinkRenderProps {
readonly isLoading: boolean
}

/** Props for a Button. */
export type ButtonProps<IconType extends string = string> =
| (BaseButtonProps<IconType, ButtonRenderProps> &
Omit<aria.ButtonProps, 'children' | 'isPending' | 'onPress'> &
Omit<AriaButtonProps, 'children' | 'isPending' | 'onPress'> &
PropsWithoutHref)
| (BaseButtonProps<IconType, LinkRenderProps> &
Omit<aria.LinkProps, 'children' | 'onPress'> &
Omit<LinkProps, 'children' | 'onPress'> &
PropsWithHref)

/** Props for a button with an href. */
Expand All @@ -68,7 +61,7 @@ export interface BaseButtonProps<IconType extends string, Render>
readonly hideLoader?: boolean
/** Falls back to `aria-label`. Pass `false` to explicitly disable the tooltip. */
readonly tooltip?: ReactElement | string | false | null
readonly tooltipPlacement?: aria.Placement
readonly tooltipPlacement?: Placement
/** The icon to display in the button */
readonly icon?: IconProp<IconType, Render>
/** When `true`, icon will be shown only when hovered. */
Expand All @@ -81,7 +74,7 @@ export interface BaseButtonProps<IconType extends string, Render>
// prettier-ignore
readonly onPress?:
// eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents
| ((event: aria.PressEvent) => Promise<unknown> | unknown)
| ((event: PressEvent) => Promise<unknown> | unknown)
| null
| undefined
readonly contentClassName?: string
Expand All @@ -100,16 +93,12 @@ export interface BaseButtonProps<IconType extends string, Render>
readonly addonEnd?: Addon<Render>
}

/**
* A type that makes all properties of a type optional
*/
/** A new type `undefined` added to all properties of a type. */
type WithUndefined<T> = {
[K in keyof T]: T[K] | undefined
}

/**
* Props that are shared between buttons in a button group.
*/
/** Props that are shared between buttons in a button group. */
export interface ButtonGroupSharedButtonProps extends WithUndefined<ButtonVariants> {
readonly isDisabled?: boolean | undefined
readonly isLoading?: boolean | undefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const DATE_PICKER_STYLES = tv({
dateInput: 'flex justify-start grow order-2',
dateSegment: 'rounded placeholder-shown:text-primary/30 focus:bg-primary/10 px-[0.5px]',
calendarButton: 'order-1 rotate-90',
resetButton: '',
resetButton: 'order-2',
calendarPopover: '',
calendarDialog: 'text-primary text-xs mx-2',
calendarContainer: '',
Expand All @@ -80,6 +80,40 @@ const DATE_PICKER_STYLES = tv({
},
})

/** Return the date segment using English placeholders. */
function normalizeDateSegment(segment: DateSegmentType): DateSegmentType {
switch (segment.type) {
case 'era': {
return { ...segment, text: 'AD', placeholder: 'AD' }
}
case 'year': {
return { ...segment, text: 'yyyy', placeholder: 'yyyy' }
}
case 'month': {
return { ...segment, text: 'mm', placeholder: 'mm' }
}
case 'day': {
return { ...segment, text: 'dd', placeholder: 'dd' }
}
case 'hour': {
return { ...segment, text: 'HH', placeholder: 'HH' }
}
case 'minute': {
return { ...segment, text: 'MM', placeholder: 'MM' }
}
case 'second': {
return { ...segment, text: 'SS', placeholder: 'SS' }
}
case 'timeZoneName': {
return { ...segment, text: 'UTC+XX', placeholder: 'UTC+XX' }
}
case 'dayPeriod':
case 'literal': {
return segment
}
}
}

/** Props for a {@link DatePicker}. */
export interface DatePickerProps<
Schema extends TSchema,
Expand Down Expand Up @@ -175,7 +209,7 @@ export const DatePicker = forwardRef(function DatePicker<
segments[segment.type] === false ?
<></>
: <DateSegment
segment={segment}
segment={normalizeDateSegment(segment)}
className={styles.dateSegment({
className:
segment.type === 'literal' && segment.text === ' ' ? 'w-1.5' : '',
Expand Down
19 changes: 14 additions & 5 deletions app/gui/src/dashboard/components/AriaComponents/Text/Text.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,21 @@ export const TEXT_STYLE = twv.tv({
// leading should always be after the text size to make sure it is not stripped by twMerge
variant: {
custom: '',
body: 'text-xs leading-[20px] before:h-[2px] after:h-[2px] macos:before:h-[1px] macos:after:h-[3px] font-medium',
body: 'text-xs leading-[20px] before:h-[2px] after:h-[2px] macos:before:h-[1px] macos:after:h-[3px',
// eslint-disable-next-line @typescript-eslint/naming-convention
'body-sm':
'text-[10.5px] leading-[16px] before:h-[2px] after:h-[2px] macos:before:h-[1px] macos:after:h-[3px] font-medium',
h1: 'text-xl leading-[29px] before:h-0.5 after:h-[5px] macos:before:h-[3px] macos:after:h-[3px] font-bold',
'text-[10.5px] leading-[16px] before:h-[2px] after:h-[2px] macos:before:h-[1px] macos:after:h-[3px]',
h1: 'text-xl leading-[29px] before:h-0.5 after:h-[5px] macos:before:h-[3px] macos:after:h-[3px]',
subtitle:
'text-[13.5px] leading-[20px] before:h-[2px] after:h-[2px] macos:before:h-[1px] macos:after:h-[3px] font-bold',
'text-[13.5px] leading-[20px] before:h-[2px] after:h-[2px] macos:before:h-[1px] macos:after:h-[3px]',
caption:
'text-[8.5px] leading-[12px] before:h-[1px] after:h-[1px] macos:before:h-[0.5px] macos:after:h-[1.5px]',
overline:
'text-[8.5px] leading-[16px] before:h-[1px] after:h-[1px] macos:before:h-[0.5px] macos:after:h-[1.5px] uppercase',
},
weight: {
custom: '',
default: '',
bold: 'font-bold',
semibold: 'font-semibold',
extraBold: 'font-extrabold',
Expand Down Expand Up @@ -117,7 +118,7 @@ export const TEXT_STYLE = twv.tv({
defaultVariants: {
variant: 'body',
font: 'default',
weight: 'medium',
weight: 'default',
transform: 'none',
color: 'primary',
italic: false,
Expand All @@ -126,6 +127,14 @@ export const TEXT_STYLE = twv.tv({
disableLineHeightCompensation: false,
textSelection: 'auto',
},
compoundVariants: [
{ variant: 'body', weight: 'default', className: 'font-medium' },
{ variant: 'body-sm', weight: 'default', className: 'font-medium' },
{ variant: 'h1', weight: 'default', className: 'font-bold' },
{ variant: 'subtitle', weight: 'default', className: 'font-bold' },
{ variant: 'caption', weight: 'default', className: 'font-medium' },
{ variant: 'overline', weight: 'default', className: 'font-medium' },
],
})

/** Text component that supports truncation and show a tooltip on hover when text is truncated */
Expand Down
44 changes: 12 additions & 32 deletions app/gui/src/dashboard/components/AriaComponents/types.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
/** @file Common types for ARIA components. */
/** @file Common types for WAI-ARIA components. */
import type { Icon as PossibleIcon } from '@/util/iconMetadata/iconName'
import type { ReactElement } from 'react'

import type * as reactAria from 'react-aria'
export type { Placement } from 'react-aria'

/** Props for adding a test id to a component */
export interface TestIdProps {
Expand All @@ -11,59 +10,40 @@ export interface TestIdProps {
readonly testId?: string | undefined
}

/**
* A type alias for the Placement type from react-aria.
* This type represents the possible positions where an element can be placed relative to a reference element.
*/
export type Placement = reactAria.Placement

/**
* Type for any icon
*/
/** Any icon. */
export type IconProp<Icon extends string = string, Render = never> =
| IconPropSvgUse<Render>
| LegacyIconProp<Icon, Render>

/**
* A type that represents the possible return values for a legacy icon.
*/
/** The possible return values for a legacy icon. */
export type LegacyAvialableIconReturn<Icon extends string> =
| LegacyIcon<Icon>
| ReactElement
| false
| null
| undefined

/**
* A type that represents the possible return values for a legacy icon.
*/
/** The possible return values for a legacy icon. */
export type AvailableIconReturn = ReactElement | SvgUseIcon | false | null | undefined

/**
* Generic type for any icon
* @deprecated Prefer defined keys over importing from `#/assets/*.svg
* Any legacy icon.
* @deprecated Prefer defined keys over importing from `#/assets/*.svg`.
*/
export type LegacyIconProp<Icon extends string, Render> =
| LegacyAvialableIconReturn<Icon>
| ((render: Render) => LegacyAvialableIconReturn<Icon>)

/**
* Generic type for imported from figma icons
*/
/** Generic type for imported from figma icons. */
export type IconPropSvgUse<Render> = AvailableIconReturn | ((render: Render) => AvailableIconReturn)

/**
* @deprecated
*/
/** @deprecated */
export type LegacyIcon<T extends string> = Exclude<T, PossibleIcon> & {}

/**
* Type for any icon imported from figma
*/
/** Any icon imported from Figma. */
export type SvgUseIcon = PossibleIcon
/**
* Generic type for any addon
*/

/** Any addon. */
export type Addon<Render> =
| ReactElement
| string
Expand Down
28 changes: 7 additions & 21 deletions app/gui/src/dashboard/components/Breadcrumbs/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
/**
* @file Breadcrumbs component implementation.
*/
/** @file A breadcrumb nagivation component. */
import { useEventCallback } from '#/hooks/eventCallbackHooks'
import { tv, type VariantProps } from '#/utilities/tailwindVariants'
import {
Expand All @@ -22,14 +20,10 @@ export const BREADCRUMBS_STYLES = tv({
slots: { separator: 'text-primary last:hidden w-2.5 h-2.5 mt-[0.5px]' },
})

/**
* The type of the `onDrop` callback.
*/
/** The type of the `onDrop` callback. */
export type OnDrop = (key: Key, e: DropEvent) => Promise<void> | void

/**
* Props for {@link Breadcrumbs}
*/
/** Props for {@link Breadcrumbs}. */
export interface BreadcrumbsProps
extends AriaBreadcrumbsProps,
VariantProps<typeof BREADCRUMBS_STYLES>,
Expand All @@ -42,9 +36,7 @@ export interface BreadcrumbsProps
readonly onDrop?: OnDrop
}

/**
* A breadcrumb navigation component.
*/
/** A breadcrumb navigation component. */
export function Breadcrumbs(props: BreadcrumbsProps) {
const {
children,
Expand Down Expand Up @@ -84,9 +76,7 @@ export function Breadcrumbs(props: BreadcrumbsProps) {
)
}

/**
* Props for {@link BreadcrumbInner}
*/
/** Props for {@link BreadcrumbInner}. */
interface BreadcrumbInnerProps extends TestIdProps, AriaBreadcrumbsProps, PropsWithChildren {
readonly className?: string
}
Expand All @@ -107,17 +97,13 @@ function BreadcrumbInner(props: BreadcrumbInnerProps) {
)
}

/**
* Props for {@link BreadcrumbSeparator}
*/
/** Props for {@link BreadcrumbSeparator}. */
interface BreadcrumbSeparatorProps<Icon extends string> {
readonly icon?: IconProp<Icon, never>
readonly className?: string
}

/**
* A separator between breadcrumb items.
*/
/** A separator between breadcrumb items. */
// eslint-disable-next-line no-restricted-syntax
const BreadcrumbSeparator = memo(function BreadcrumbSeparator<Icon extends string>(
props: BreadcrumbSeparatorProps<Icon>,
Expand Down
4 changes: 1 addition & 3 deletions app/gui/src/dashboard/components/Icon/Icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ export interface LegacyIconProps<Icon extends string, Render = never>
readonly icon?: never
}

/**
* Generic type for imported from figma icons
*/
/** Generic type for icons imported from Figma. */
export interface SvgUseIconProps<Render = never> {
readonly children?: never
readonly icon: IconTypeSvgUse<Render>
Expand Down
Loading
Loading