Skip to content

Commit

Permalink
chore(lemon-ui): Make scrollable popovers sleek (#19973)
Browse files Browse the repository at this point in the history
* Make scrollable popovers sleek

* Remove the `actionable` popover prop

* `forwardRef` to `ScrollableShadows`

* Fix `LemonMenu` story

* Round popover offset

* Update UI snapshots for `chromium` (2)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
Twixes and github-actions[bot] authored Jan 26, 2024
1 parent f447507 commit ef1d7d1
Show file tree
Hide file tree
Showing 19 changed files with 40 additions and 92 deletions.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-menu--flat--dark.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-menu--nested-menu--light.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ export const breadcrumbsLogic = kea<breadcrumbsLogicType>([
otherOrganizations?.length || preflight?.can_create_org
? {
overlay: <OrganizationSwitcherOverlay />,
actionable: true,
}
: undefined,
})
Expand All @@ -152,7 +151,6 @@ export const breadcrumbsLogic = kea<breadcrumbsLogicType>([
name: currentTeam.name,
popover: {
overlay: <ProjectSwitcherOverlay />,
actionable: true,
},
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,11 @@
.LemonModal {
min-width: var(--annotations-popover-width);
max-width: var(--annotations-popover-width);
min-height: 100%;
margin: 0;
border: none;
box-shadow: none;
}

.Popover__box {
padding: 0;
}

ul {
max-height: calc(4 * 6.75rem + 3 * 0.5rem); // Fit in 4 one-line annotations without scrolling
}
}

.AnnotationCard {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ function AnnotationsPopover({
visible={isPopoverShown}
onClickOutside={closePopover}
showArrow
padded={false}
overlay={
<LemonModal
inline
Expand Down
1 change: 0 additions & 1 deletion frontend/src/lib/components/HelpButton/HelpButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ export function HelpButton({
onVisibilityChange={(visible) => !visible && hideHelp()}
visible={isHelpVisible}
placement={placement}
actionable
onClickOutside={hideHelp}
>
<div className={clsx('help-button', inline && 'inline')} onClick={toggleHelp} data-attr="help-button">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.filter-row-popover .Popover__box {
.filter-row-popover .ScrollableShadows {
overflow: visible; // Only required because the Ant popover is rendered _within_ the filter popover
}

Expand Down
30 changes: 17 additions & 13 deletions frontend/src/lib/components/ScrollableShadows/ScrollableShadows.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import './ScrollableShadows.scss'

import { clsx } from 'clsx'
import { useScrollable } from 'lib/hooks/useScrollable'
import { MutableRefObject } from 'react'
import React, { MutableRefObject } from 'react'

export type ScrollableShadowsProps = {
children: React.ReactNode
Expand All @@ -12,14 +12,17 @@ export type ScrollableShadowsProps = {
scrollRef?: MutableRefObject<HTMLDivElement | null>
}

export const ScrollableShadows = ({
children,
direction,
className,
innerClassName,
scrollRef,
}: ScrollableShadowsProps): JSX.Element => {
const { ref, isScrollableLeft, isScrollableRight, isScrollableBottom, isScrollableTop } = useScrollable()
export const ScrollableShadows = React.forwardRef<HTMLDivElement, ScrollableShadowsProps>(function ScrollableShadows(
{ children, direction, className, innerClassName, scrollRef },
ref
) {
const {
ref: scrollRefScrollable,
isScrollableLeft,
isScrollableRight,
isScrollableBottom,
isScrollableTop,
} = useScrollable()

return (
<div
Expand All @@ -33,18 +36,19 @@ export const ScrollableShadows = ({
direction === 'vertical' && isScrollableBottom && 'ScrollableShadows--bottom',
className
)}
ref={ref}
>
<div
className={clsx('ScrollableShadows__inner', innerClassName)}
ref={(theRef) => {
ref.current = theRef
ref={(refValue) => {
scrollRefScrollable.current = refValue
if (scrollRef) {
scrollRef.current = theRef
scrollRef.current = refValue
}
}}
>
{children}
</div>
</div>
)
}
})
13 changes: 2 additions & 11 deletions frontend/src/lib/lemon-ui/LemonMenu/LemonMenu.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,8 @@ export default meta

const Template: StoryFn<typeof LemonMenuOverlayComponent> = (props: LemonMenuOverlayProps) => {
return (
<div className="Popover">
<div
className="Popover__box"
// eslint-disable-next-line react/forbid-dom-props
style={{
opacity: 1,
width: 'fit-content',
}}
>
<LemonMenuOverlayComponent {...props} />
</div>
<div className="rounded border p-1 bg-bg-light">
<LemonMenuOverlayComponent {...props} />
</div>
)
}
Expand Down
4 changes: 1 addition & 3 deletions frontend/src/lib/lemon-ui/LemonMenu/LemonMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ export interface LemonMenuProps
LemonDropdownProps,
| 'placement'
| 'fallbackPlacements'
| 'actionable'
| 'sameWidth'
| 'maxContentWidth'
| 'visible'
Expand Down Expand Up @@ -167,7 +166,7 @@ export function LemonMenuSectionList({
<section className="space-y-px">
{section.title ? (
typeof section.title === 'string' ? (
<h5>{section.title}</h5>
<h5 className="mx-2 my-1">{section.title}</h5>
) : (
section.title
)
Expand Down Expand Up @@ -265,7 +264,6 @@ const LemonMenuItemButton: FunctionComponent<LemonMenuItemButtonProps & React.Re
items={items}
tooltipPlacement={tooltipPlacement}
placement="right-start"
actionable
closeOnClickInside={custom ? false : true}
closeParentPopoverOnClickInside={custom ? false : true}
>
Expand Down
1 change: 0 additions & 1 deletion frontend/src/lib/lemon-ui/LemonSelect/LemonSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ export function LemonSelect<T extends string | number | boolean | null>({
tooltipPlacement={optionTooltipPlacement}
sameWidth={dropdownMatchSelectWidth}
placement={dropdownPlacement}
actionable
className={menu?.className}
maxContentWidth={dropdownMaxContentWidth}
activeItemIndex={items
Expand Down
55 changes: 11 additions & 44 deletions frontend/src/lib/lemon-ui/Popover/Popover.scss
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,14 @@
position: relative; // For arrow
flex-grow: 1;
max-width: 100%;
padding: 0.5rem;
overflow: hidden;
background: var(--bg-light);
border: 1px solid var(--border);
border: 1px solid var(--secondary-3000-button-border);
border-radius: var(--radius);
box-shadow: var(--shadow-elevation);
opacity: 0;
transition: opacity 50ms ease, transform 50ms ease;
transform-origin: top;

.Popover--actionable & {
border-color: var(--primary-3000);
}

// We set the offset below instead of using floating-ui's offset(), because we need there to be no gap between
// the reference and the floating element. This makes hover-based popovers possible

Expand Down Expand Up @@ -94,72 +88,45 @@
.Popover--max-content-width & {
width: max-content;
}

.posthog-3000 &,
.posthog-3000 .Popover--actionable & {
padding: 0.25rem;
background: var(--bg-light);
border-color: var(--secondary-3000-button-border);
}
}

.Popover__arrow {
position: absolute;
width: 0.5rem;
height: 0.5rem;
background: var(--bg-3000);
background: var(--bg-light);
transform: rotate(45deg);

.posthog-3000 & {
background: var(--bg-light);
}

[data-placement^='bottom'] & {
top: -0.25rem;
top: -0.3125rem;
border-top: 1px solid var(--border);
border-left: 1px solid var(--border);

.posthog-3000 & {
top: -0.3rem;
}
}

[data-placement^='top'] & {
bottom: -0.25rem;
bottom: -0.3125rem;
border-right: 1px solid var(--border);
border-bottom: 1px solid var(--border);

.posthog-3000 & {
bottom: -0.3rem;
}
}

[data-placement^='left'] & {
right: -0.25rem;
right: -0.3125rem;
border-top: 1px solid var(--border);
border-right: 1px solid var(--border);

.posthog-3000 & {
right: -0.3rem;
}
}

[data-placement^='right'] & {
left: -0.25rem;
left: -0.3125rem;
border-bottom: 1px solid var(--border);
border-left: 1px solid var(--border);

.posthog-3000 & {
left: -0.3rem;
}
}

.Popover--actionable & {
border-color: var(--primary-3000);
}
}

.Popover__content {
max-height: 100%;
overflow-y: auto;
border-radius: calc(var(--radius) - 1px);
}

.Popover--padded .Popover__content > .ScrollableShadows__inner {
padding: 0.25rem;
}
10 changes: 5 additions & 5 deletions frontend/src/lib/lemon-ui/Popover/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
useMergeRefs,
} from '@floating-ui/react'
import clsx from 'clsx'
import { ScrollableShadows } from 'lib/components/ScrollableShadows/ScrollableShadows'
import { useEventListener } from 'lib/hooks/useEventListener'
import { useFloatingContainerContext } from 'lib/hooks/useFloatingContainerContext'
import { CLICK_OUTSIDE_BLOCK_CLASS, useOutsideClickHandler } from 'lib/hooks/useOutsideClickHandler'
Expand Down Expand Up @@ -87,7 +88,6 @@ export const Popover = React.forwardRef<HTMLDivElement, PopoverProps>(function P
fallbackPlacements = ['bottom-start', 'bottom-end', 'top-start', 'top-end'],
className,
padded = true,
actionable = false,
middleware,
sameWidth = false,
maxContentWidth = false,
Expand Down Expand Up @@ -225,7 +225,7 @@ export const Popover = React.forwardRef<HTMLDivElement, PopoverProps>(function P
<div
className={clsx(
'Popover',
actionable && 'Popover--actionable',
padded && 'Popover--padded',
maxContentWidth && 'Popover--max-content-width',
!isAttached && 'Popover--top-centered',
showArrow && 'Popover--with-arrow',
Expand All @@ -252,7 +252,7 @@ export const Popover = React.forwardRef<HTMLDivElement, PopoverProps>(function P
onMouseLeave={onMouseLeaveInside}
aria-level={currentPopoverLevel}
>
<div className={clsx('Popover__box', !padded && 'p-0')}>
<div className="Popover__box">
{showArrow && isAttached && (
// Arrow is outside of .Popover__content to avoid affecting :nth-child for content
<div
Expand All @@ -262,9 +262,9 @@ export const Popover = React.forwardRef<HTMLDivElement, PopoverProps>(function P
style={arrowStyle}
/>
)}
<div className="Popover__content" ref={contentRef}>
<ScrollableShadows className="Popover__content" ref={contentRef} direction="vertical">
{overlay}
</div>
</ScrollableShadows>
</div>
</div>
</PopoverOverlayContext.Provider>
Expand Down
1 change: 0 additions & 1 deletion frontend/src/scenes/notebooks/NotebookMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ export function NotebookMenu({ shortId }: NotebookLogicProps): JSX.Element {
},
},
]}
actionable
>
<LemonButton aria-label="more" icon={<IconEllipsis />} size="small" />
</LemonMenu>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ export function NotebooksTable(): JSX.Element {
},
},
]}
actionable
>
<LemonButton aria-label="more" icon={<IconEllipsis />} size="small" />
</LemonMenu>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2895,7 +2895,7 @@ interface BreadcrumbBase {
/** Symbol, e.g. a lettermark or a profile picture. */
symbol?: React.ReactNode
/** Whether to show a custom popover */
popover?: Pick<PopoverProps, 'overlay' | 'sameWidth' | 'actionable'>
popover?: Pick<PopoverProps, 'overlay' | 'sameWidth'>
}
interface LinkBreadcrumb extends BreadcrumbBase {
/** Path to link to. */
Expand Down

0 comments on commit ef1d7d1

Please sign in to comment.