Skip to content

Conversation

@IvanKalinin
Copy link

@IvanKalinin IvanKalinin commented Oct 29, 2022

In this pull request I introduce 2 Dialog props:

  1. closeOnEsc - by default is true. Does not change the current behavior.
    When it is false it disables closing the Dialog by pressing the Escape button.
    Useful when we want users to close a Dialog only by clicking a cancel button. E.g. when we render a form and don't want to lose the data in the form by accidentally pressing Esc.

  2. closeOnOutsideClick - by default is true. Does not change the current behavior.
    When it is false it disables closing the Dialog on the outside click. As useful as clickOnEsc. Also, it addressed the issues

#1700
#1751

@vercel
Copy link

vercel bot commented Oct 29, 2022

@IvanKalinin is attempting to deploy a commit to the Tailwind Labs Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 29, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
headlessui-react ✅ Ready (Inspect) Visit Preview Oct 29, 2022 at 5:47AM (UTC)

@adamwathan
Copy link
Member

Hey thanks for the PR! I do like the idea of providing more control over this behavior but am not totally convinced yet that this is the right API so I’m going to close this as-is.

Unfortunately can’t make it a priority to think through this particular feature or decide on any APIs right now so I’m not able to provide any specific feedback or suggestions on what to do differently to increase the chances of it being merged.

Thanks regardless, I appreciate the contribution 👍🏻

@mgambill
Copy link

What if we just pass information as part of the emit event then the user will have information to determine if they want to close or not. @adamwathan

api.close( reason ) // 'backdrop' | 'key' | 'trap' | 'other')

function onClose(e) {
   if (e.reason === 'backdrop') // update ref or not 
}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants