Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
@@ -1,4 +1,5 @@
@import "~@vibe/style/dist/mixins";
@import "~@vibe/style/dist/mixins/motion";

.contentWrapper {
outline: 0;
Expand Down Expand Up @@ -53,121 +54,147 @@
}

// Animations
//
// Two flavors driven by Dialog's `animationType` prop:
// - opacity-and-slide: fade + 16px translate from origin (default for popovers)
// - expand: scale-from-origin + fade (used by Tooltip, Menu submenus)
//
// Both are gated on `prefers-reduced-motion: no-preference`. Reduced-motion
// users get the popover at rest immediately with no transition.

$translate-minus-px: calc(var(--space-16) * -1);

.opacitySlideAppear {
opacity: 0;
@include motion-safe {
opacity: 0;

&.top {
transform: translateY(var(--space-16));
}
&.top {
transform: translateY(var(--space-16));
}

&.right {
transform: translateX($translate-minus-px);
}
&.right {
transform: translateX($translate-minus-px);
}

&.bottom {
transform: translateY($translate-minus-px);
}
&.bottom {
transform: translateY($translate-minus-px);
}

&.left {
transform: translateX(var(--space-16));
&.left {
transform: translateX(var(--space-16));
}
}
}

.opacitySlideAppearActive {
transition: opacity 0.2s ease, transform 0.2s ease-out;
opacity: 1;
pointer-events: none;

Comment on lines 89 to 91

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

1. Reduced-motion blocks clicks 🐞 Bug ≡ Correctness

In DialogContent animations, pointer-events: none is applied outside motion-safe, so with
prefers-reduced-motion: reduce the popover can mount fully visible but remain non-interactive for
the full CSSTransition timeout. This is user-visible on click-trigger dialogs that rely on the
default showDelay timeout (e.g. Info).
Agent Prompt
### Issue description
`pointer-events: none` is currently applied unconditionally on `*.AppearActive` classes. When `prefers-reduced-motion: reduce` is set, the `motion-safe` blocks don’t apply (no transition/opacity/transform rules), but the CSSTransition classes still apply and keep the content non-interactive for the whole timeout.

### Issue Context
- `DialogContent.tsx` always runs `CSSTransition` with `timeout={showDelay}`.
- Reduced-motion users should get the popover “at rest immediately”, which also implies it should be interactive immediately.

### Fix Focus Areas
- packages/components/dialog/src/Dialog/components/DialogContent/DialogContent.module.scss[89-106]
- packages/components/dialog/src/Dialog/components/DialogContent/DialogContent.module.scss[177-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

&.top,
&.bottom {
transform: translateY(0);
}
@include motion-safe {
transition: opacity var(--motion-productive-long) var(--motion-timing-enter),
transform var(--motion-productive-long) var(--motion-timing-enter);
opacity: 1;

&.right,
&.left {
transform: translateX(0);
&.top,
&.bottom {
transform: translateY(0);
}

&.right,
&.left {
transform: translateX(0);
}
}
}

.expandAppear,
.expandExit {
transition: transform 0.1s $expand-animation-timing;
&.top,
&.topStart,
&.topEnd {
transform-origin: bottom center;
transform: scale(0.8);
&.edgeBottom {
transform-origin: bottom left;
}
&.edgeTop {
transform-origin: bottom right;
@include motion-safe {
transition: transform var(--motion-productive-long) var(--motion-timing-enter),
opacity var(--motion-productive-long) var(--motion-timing-enter);
opacity: 0;
Comment on lines +111 to +114

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

2. Dialogcontent timeout mismatch 🐞 Bug ≡ Correctness

DialogContent’s CSS now uses --motion-productive-long (150ms) for expand transitions, but
DialogContent.tsx still ends the transition after timeout={showDelay} (default 100ms). This
removes the active transition classes early, truncating the animation and making the new token
timing not actually take effect.
Agent Prompt
### Issue description
The JS-driven transition lifecycle duration (CSSTransition `timeout`) is shorter than the new CSS transition duration (150ms). This causes class removal before the CSS transition completes, effectively cutting animations short.

### Issue Context
- `Dialog.tsx` already uses `showDelay` to delay opening; `showDelay` should not also be used as the animation duration.
- Motion tokens define `--motion-productive-long: 150ms`.

### Fix Focus Areas
- packages/components/dialog/src/Dialog/components/DialogContent/DialogContent.tsx[185-233]
- packages/components/dialog/src/Dialog/Dialog.tsx[33-45]
- packages/components/dialog/src/Dialog/components/DialogContent/DialogContent.module.scss[109-114]
- packages/style/src/motion.scss[1-6]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


&.top,
&.topStart,
&.topEnd {
transform-origin: bottom center;
transform: scale(0.92);
&.edgeBottom {
transform-origin: bottom left;
}
&.edgeTop {
transform-origin: bottom right;
}
}
}

&.right,
&.rightStart,
&.rightEnd {
transform-origin: left;
transform: scale(0.8);
&.edgeBottom {
transform-origin: top left;
}
&.edgeTop {
transform-origin: bottom left;
&.right,
&.rightStart,
&.rightEnd {
transform-origin: left;
transform: scale(0.92);
&.edgeBottom {
transform-origin: top left;
}
&.edgeTop {
transform-origin: bottom left;
}
}
}

&.bottom,
&.bottomStart,
&.bottomEnd {
transform-origin: top;
transform: scale(0.8);
&.edgeBottom {
transform-origin: top left;
&.bottom,
&.bottomStart,
&.bottomEnd {
transform-origin: top;
transform: scale(0.92);
&.edgeBottom {
transform-origin: top left;
}
&.edgeTop {
transform-origin: top right;
}
}
&.edgeTop {
transform-origin: top right;
}
}

&.left,
&.leftStart,
&.leftEnd {
transform-origin: right;
transform: scale(0.8);
&.edgeBottom {
transform-origin: top right;
}
&.edgeTop {
transform-origin: bottom right;
&.left,
&.leftStart,
&.leftEnd {
transform-origin: right;
transform: scale(0.92);
&.edgeBottom {
transform-origin: top right;
}
&.edgeTop {
transform-origin: bottom right;
}
}
}
}

.expandExit {
transition: transform 0.1s $expand-animation-timing;
@include motion-safe {
transition: transform var(--motion-productive-medium) var(--motion-timing-exit),
opacity var(--motion-productive-medium) var(--motion-timing-exit);
}
}

.expandAppearActive {
transition: transform 0.1s $expand-animation-timing;
pointer-events: none;

&.top,
&.topStart,
&.topEnd,
&.bottom,
&.bottomStart,
&.bottomEnd,
&.right,
&.rightStart,
&.rightEnd,
&.left,
&.leftStart,
&.leftEnd {
transform: scale(1);
@include motion-safe {
transition: transform var(--motion-productive-long) var(--motion-timing-enter),
opacity var(--motion-productive-long) var(--motion-timing-enter);
opacity: 1;

&.top,
&.topStart,
&.topEnd,
&.bottom,
&.bottomStart,
&.bottomEnd,
&.right,
&.rightStart,
&.rightEnd,
&.left,
&.leftStart,
&.leftEnd {
transform: scale(1);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
@import "~@vibe/style/dist/mixins";
@import "~@vibe/style/dist/mixins/motion";

// Submenu open/close animation. Mirrors the `expand` variant in DialogContent —
// fade + scale-from-origin keyed off the floating-ui placement so the submenu
// grows out of the parent menu item's edge.
//
// Uses CSS animations instead of transitions because CSSTransition+mountOnEnter
// can skip the "from" frame when the class swap lands in a single paint —
// keyframes fire reliably on a freshly mounted element.

@keyframes submenuExpandIn {
from {
opacity: 0;
transform: scale(0.92);
}
to {
opacity: 1;
transform: scale(1);
}
}

@keyframes submenuExpandOut {
from {
opacity: 1;
transform: scale(1);
}
to {
opacity: 0;
transform: scale(0.92);
}
}

.rightStart {
transform-origin: top left;
}

.rightEnd {
transform-origin: bottom left;
}

.leftStart {
transform-origin: top right;
}

.leftEnd {
transform-origin: bottom right;
}

.appearActive,
.enterActive {
@include motion-safe {
animation: submenuExpandIn var(--motion-productive-long) var(--motion-timing-enter) forwards;
}
}

.exitActive {
@include motion-safe {
animation: submenuExpandOut var(--motion-productive-medium) var(--motion-timing-exit) forwards;
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import React, { useMemo, useRef } from "react";
import { CSSTransition } from "react-transition-group";
import { camelCase } from "es-toolkit";
import { DialogContentContainer } from "@vibe/dialog";
import { useFloating, flip, type Placement } from "@floating-ui/react-dom";
import { type MenuChild } from "../../../Menu/MenuConstants";
import { type MenuItemSubMenuProps } from "./MenuItemSubMenu.types";
import { useIsomorphicLayoutEffect } from "@vibe/shared";
import { useIsomorphicLayoutEffect, getStyle } from "@vibe/shared";
import styles from "./MenuItemSubMenu.module.scss";

const DEFAULT_FALLBACK_PLACEMENTS: Placement[] = ["right-end", "left-start", "left-end"];

Expand All @@ -16,6 +19,7 @@ const MenuItemSubMenu = ({
submenuPosition
}: MenuItemSubMenuProps) => {
const childRef = useRef<HTMLDivElement>(null);
const transitionRef = useRef<HTMLDivElement>(null);

useIsomorphicLayoutEffect(() => {
if (!autoFocusOnMount || !open || !childRef?.current) {
Expand Down Expand Up @@ -49,24 +53,36 @@ const MenuItemSubMenu = ({
return null;
}

const placementClassName = getStyle(styles, camelCase(actualPlacement));

return (
<div
style={{ ...floatingStyles, visibility: open ? "visible" : "hidden" }}
ref={refs.setFloating}
data-popper-placement={actualPlacement}
>
{subMenu && open && (
<DialogContentContainer>
{React.cloneElement(subMenu, {
...subMenu?.props,
isVisible: open,
isSubMenu: true,
onClose,
ref: childRef,
useDocumentEventListeners: !autoFocusOnMount
})}
</DialogContentContainer>
)}
<div style={floatingStyles} ref={refs.setFloating} data-popper-placement={actualPlacement}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

1. menuitemsubmenu root lacks data-vibe 📘 Rule violation ◔ Observability

The updated root element for MenuItemSubMenu does not include the required [data-vibe]
attribute. This breaks the standard component identification/instrumentation requirement.
Agent Prompt
## Issue description
`MenuItemSubMenu`'s root rendered element is missing the required `[data-vibe]` attribute.

## Issue Context
Compliance requires all React component roots to include `[data-vibe]` for consistent identification/instrumentation.

## Fix Focus Areas
- packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenu/MenuItemSubMenu.tsx[58-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

<CSSTransition
in={open}
appear
mountOnEnter
unmountOnExit
nodeRef={transitionRef}
timeout={{ appear: 150, enter: 150, exit: 100 }}
classNames={{
appearActive: styles.appearActive,
enterActive: styles.enterActive,
exitActive: styles.exitActive
}}
Comment on lines +60 to +71

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

1. Submenu enter flash risk 🐞 Bug ≡ Correctness

MenuItemSubMenu’s CSSTransition only provides *Active classNames (no appear/enter base classes), so
the submenu can mount at its resting (visible) styles for a paint and then snap to the keyframe
from state when enterActive is applied, producing a visible flash on open. DialogContent avoids
this by mapping an explicit appear class that sets the initial hidden/scale state before the
active transition begins.
Agent Prompt
### Issue description
`MenuItemSubMenu` wires `CSSTransition` with only `appearActive`/`enterActive`/`exitActive`. Because there is no `appear`/`enter` base class that sets the initial hidden/scale state, the submenu element can render once at its default (opacity 1 / scale 1) before the active class is applied, then immediately jump to the keyframe `from` state (opacity 0 / scale 0.92), causing a flash.

### Issue Context
This code intentionally uses keyframes to avoid missing the “from” frame on mount, but it still needs an initial base class (like DialogContent’s `expandAppear`) to ensure the element is already in the correct starting state before the `*Active` class is applied.

### Fix Focus Areas
- packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenu/MenuItemSubMenu.tsx[58-72]
- packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenu/MenuItemSubMenu.module.scss[50-61]

### Suggested fix
1. Add base class mappings in `CSSTransition.classNames`, e.g. `appear: styles.appear`, `enter: styles.enter` (and keep existing `appearActive`/`enterActive`/`exitActive`).
2. In the SCSS module, add `.appear` and `.enter` rules (under `@include motion-safe`) that set the initial state to match your keyframe `from` values (e.g. `opacity: 0; transform: scale(0.92);`).
3. Keep the placement transform-origin classes as-is (they only set `transform-origin`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

>
<div ref={transitionRef} className={placementClassName}>
<DialogContentContainer>
{React.cloneElement(subMenu, {
...subMenu?.props,
isVisible: open,
isSubMenu: true,
onClose,
ref: childRef,
useDocumentEventListeners: !autoFocusOnMount
})}
</DialogContentContainer>
</div>
</CSSTransition>
</div>
);
};
Expand Down
Loading
Loading