Skip to content

fix(Dialog): click outside handler should be on the wrapper - not content - of the Dialog #3145

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 22, 2025
Merged

fix(Dialog): click outside handler should be on the wrapper - not content - of the Dialog #3145

merged 3 commits into from
Oct 22, 2025

Conversation

@YossiSaadi
Copy link
Contributor

@YossiSaadi YossiSaadi commented Oct 21, 2025 •
edited
Loading

This caused cases where when we used LayerProvider to send other dialogs to live under our root dialog (like rendering a dropdown inside a dialog), we got a click outside trigger everytime a user chose something on the dropdown.
this should fix it.

https://monday.monday.com/boards/3532714909/pulses/18233338168

@github-actions
Copy link
Contributor

github-actions bot commented Oct 21, 2025

A new prerelease version of this PR has been published! 🎉
To install this prerelease version, run the following command in your terminal with any one of the packages changed in this PR:

To update @vibe/core:

yarn add @vibe/[email protected]

Or with npm:

npm i @vibe/[email protected]

rivka-ungar
rivka-ungar approved these changes Oct 21, 2025
View reviewed changes
Copy link
Contributor

@rivka-ungar rivka-ungar left a comment

Choose a reason for hiding this comment

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

👑 👑 👑

@github-actions
Copy link
Contributor

github-actions bot commented Oct 21, 2025

📦 Bundle Size Analysis

✅ No bundle size changes detected.

Unchanged Components
Component Base PR Diff
Accordion 6.35KB 6.35KB 0B ➖
AccordionItem 67.85KB 67.79KB -69B 🟢
AlertBanner 72.49KB 72.45KB -43B 🟢
AlertBannerButton 19.03KB 19.03KB 0B ➖
AlertBannerLink 15.35KB 15.35KB 0B ➖
AlertBannerText 65.12KB 65.21KB +89B 🔺
AttentionBox 74KB 73.97KB -28B 🟢
AttentionBoxLink 15.22KB 15.22KB 0B ➖
Avatar 68.02KB 68.01KB -6B 🟢
AvatarGroup 95.4KB 95.37KB -27B 🟢
Badge 43.55KB 43.55KB 0B ➖
Box 9.31KB 9.31KB 0B ➖
BreadcrumbItem 65.91KB 65.78KB -125B 🟢
BreadcrumbMenu 69.95KB 69.96KB +6B 🔺
BreadcrumbMenuItem 78.72KB 78.61KB -119B 🟢
BreadcrumbsBar 5.76KB 5.76KB 0B ➖
Button 18.73KB 18.73KB 0B ➖
ButtonGroup 69.93KB 69.94KB +4B 🔺
Checkbox 68.11KB 68.06KB -48B 🟢
Chips 76.8KB 76.66KB -140B 🟢
Clickable 6.27KB 6.27KB 0B ➖
ColorPicker 75.9KB 75.87KB -32B 🟢
ColorPickerContent 75.14KB 75.22KB +78B 🔺
Combobox 85.55KB 85.58KB +35B 🔺
Counter 42.45KB 42.45KB 0B ➖
DatePicker 134.19KB 134.19KB 0B ➖
Dialog 53.98KB 54.04KB +56B 🔺
DialogContentContainer 6.3KB 6.3KB 0B ➖
Divider 5.54KB 5.54KB 0B ➖
Dropdown 125.17KB 125.14KB -32B 🟢
menu 59.93KB 59.93KB 0B ➖
option 92.85KB 92.78KB -79B 🟢
singleValue 92.74KB 92.73KB -3B 🟢
EditableHeading 67.78KB 67.8KB +12B 🔺
EditableText 67.77KB 67.67KB -102B 🟢
EmptyState 72.39KB 72.38KB -7B 🟢
ExpandCollapse 67.6KB 67.59KB -7B 🟢
Flex 7.46KB 7.46KB 0B ➖
FormattedNumber 5.87KB 5.87KB 0B ➖
GridKeyboardNavigationContext 4.65KB 4.65KB 0B ➖
Heading 66.52KB 66.43KB -93B 🟢
HiddenText 5.41KB 5.41KB 0B ➖
CustomSvgIcon 11.86KB 11.86KB 0B ➖
Icon 13.82KB 13.82KB 0B ➖
IconButton 69.65KB 69.65KB +6B 🔺
Info 73.81KB 73.93KB +119B 🔺
Label 69.96KB 70.03KB +72B 🔺
LayerProvider 2.95KB 2.95KB 0B ➖
LegacyModal 76.35KB 76.36KB +16B 🔺
LegacyModalContent 66.55KB 66.57KB +28B 🔺
LegacyModalFooter 3.47KB 3.47KB 0B ➖
LegacyModalFooterButtons 20.46KB 20.46KB 0B ➖
LegacyModalHeader 72.49KB 72.46KB -28B 🟢
Link 15KB 15KB 0B ➖
List 74.38KB 74.42KB +41B 🔺
ListItem 67.1KB 67.13KB +31B 🔺
ListItemAvatar 68.13KB 68.18KB +57B 🔺
ListItemIcon 14.12KB 14.12KB 0B ➖
ListTitle 66.56KB 66.57KB +6B 🔺
Loader 5.85KB 5.85KB 0B ➖
Menu 8.66KB 8.66KB 0B ➖
MenuDivider 5.66KB 5.66KB 0B ➖
MenuGridItem 7.22KB 7.22KB 0B ➖
MenuItem 78.7KB 78.75KB +43B 🔺
MenuItemButton 71.7KB 71.85KB +157B 🔺
MenuTitle 66.83KB 66.88KB +55B 🔺
MenuButton 67.56KB 67.54KB -20B 🟢
MultiStepIndicator 53KB 53KB 0B ➖
NumberField 74.53KB 74.5KB -29B 🟢
LinearProgressBar 7.42KB 7.42KB 0B ➖
RadioButton 67.32KB 67.37KB +45B 🔺
Search 72.05KB 72.05KB -1B 🟢
Skeleton 6.2KB 6.2KB 0B ➖
Slider 75.64KB 75.61KB -32B 🟢
SplitButton 68.35KB 68.37KB +20B 🔺
SplitButtonMenu 8.82KB 8.82KB 0B ➖
Steps 73.05KB 73.09KB +44B 🔺
Table 7.32KB 7.32KB 0B ➖
TableBody 68.43KB 68.3KB -137B 🟢
TableCell 66.63KB 66.59KB -35B 🟢
TableContainer 5.38KB 5.38KB 0B ➖
TableHeader 5.7KB 5.7KB 0B ➖
TableHeaderCell 73.67KB 73.85KB +179B 🔺
TableRow 5.58KB 5.58KB 0B ➖
TableRowMenu 70.31KB 70.24KB -74B 🟢
TableVirtualizedBody 72.96KB 72.94KB -28B 🟢
Tab 65.21KB 65.23KB +26B 🔺
TabList 8.92KB 8.92KB 0B ➖
TabPanel 5.36KB 5.36KB 0B ➖
TabPanels 5.91KB 5.91KB 0B ➖
TabsContext 5.54KB 5.54KB 0B ➖
Text 66.36KB 66.39KB +36B 🔺
TextArea 67.63KB 67.71KB +84B 🔺
TextField 70.96KB 70.91KB -50B 🟢
TextWithHighlight 65.53KB 65.54KB +19B 🔺
ThemeProvider 4.69KB 4.69KB 0B ➖
Tipseen 72.91KB 72.87KB -37B 🟢
TipseenContent 73.33KB 73.3KB -31B 🟢
TipseenImage 73.15KB 73.15KB -2B 🟢
TipseenMedia 73.03KB 73.12KB +92B 🔺
TipseenWizard 75.6KB 75.58KB -18B 🟢
Toast 75.67KB 75.71KB +32B 🔺
ToastButton 18.93KB 18.93KB 0B ➖
ToastLink 15.22KB 15.22KB 0B ➖
Toggle 68.06KB 68.04KB -13B 🟢
Tooltip 64.41KB 64.42KB +10B 🔺
TransitionView 37.65KB 37.65KB 0B ➖
VirtualizedGrid 12.58KB 12.58KB 0B ➖
VirtualizedList 12.34KB 12.34KB 0B ➖

📊 Summary:

  • Total Base Size: 5.12MB
  • Total PR Size: 5.12MB
  • Total Difference: +3B

@YossiSaadi YossiSaadi marked this pull request as ready for review October 22, 2025 09:47
@YossiSaadi YossiSaadi requested a review from a team as a code owner October 22, 2025 09:47
@YossiSaadi YossiSaadi merged commit b154daf into master Oct 22, 2025
17 checks passed
@YossiSaadi YossiSaadi deleted the fix/yossi/dialog-when-rendering-a-dropdown-or-any-other-dialog-based-18233338168 branch October 22, 2025 09:47
@qodo-merge-for-open-source
Copy link
Contributor

qodo-merge-for-open-source bot commented Oct 22, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
 Summary: ⚠️ Title violates Conventional Commits.
⚡ Recommended focus areas for review

Ref Semantics

Verify that changing the forwarded ref target from the content div to the wrapper span does not break consumers that depended on measuring or focusing the inner content element.

 
<span
  // don't remove old classname - override from Monolith
  className={cx("monday-style-dialog-content-wrapper", styles.contentWrapper, wrapperClassName)}
  ref={mergedWrapperRef}
  data-testid={dataTestId}
  style={styleObject}
  onClickCapture={onClick}
  data-popper-reference-hidden={isReferenceHidden}
>
  <CSSTransition
    classNames={transitionOptions.classNames}
    nodeRef={contentRef}
    in={isOpen}
    appear={!!animationType}
Click Outside Scope

Ensure attaching useClickOutside to the wrapper span correctly includes all interactive areas (padding, arrows) but does not capture clicks inside nested dialogs or portals unintentionally.

 
// Watch the wrapper ref to include padding area, tooltip arrows, and nested Dialogs
useClickOutside({ callback: onOutSideClick, ref: wrapperRef });
useClickOutside({ eventName: "contextmenu", callback: overrideOnContextMenu, ref: wrapperRef });
const selectorToDisable = typeof disableContainerScroll === "string" ? disableContainerScroll : containerSelector;
Transition NodeRef

Confirm that CSSTransition’s nodeRef now pointing to the content div aligns with how the transition classes are applied and that mounting/unmounting behavior remains correct.

 
<CSSTransition
  classNames={transitionOptions.classNames}
  nodeRef={contentRef}
  in={isOpen}
  appear={!!animationType}
  timeout={showDelay}
>
  <div
    className={cx(styles.contentComponent, getStyle(styles, camelCase(position)), {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rivka-ungar rivka-ungar rivka-ungar approved these changes

Assignees

No one assigned

Labels

Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@YossiSaadi @rivka-ungar