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

feat: isConfirmDangerProp #1110

Merged
merged 3 commits into from
Feb 27, 2023

Conversation

bmingles
Copy link
Contributor

resolves #1109

@bmingles bmingles changed the title feature: isConfirmDangerProp feat: isConfirmDangerProp Feb 24, 2023
@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Merging #1110 (d652ffd) into main (144605a) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1110      +/-   ##
==========================================
+ Coverage   42.91%   42.93%   +0.02%     
==========================================
  Files         434      434              
  Lines       32596    32598       +2     
  Branches     8202     8204       +2     
==========================================
+ Hits        13989    13997       +8     
+ Misses      18558    18552       -6     
  Partials       49       49              
Flag Coverage Δ
unit 42.93% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/components/src/BasicModal.tsx 58.33% <100.00%> (+8.33%) ⬆️
packages/components/src/Button.tsx 76.31% <0.00%> (+5.26%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -64,8 +67,12 @@ function BasicModal(props: BasicModalProps) {
}, [onConfirm, onModalDisable]);

const onOpened = useCallback(() => {
confirmButton.current?.focus();
}, []);
if (isConfirmDanger) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably trust the user and keep the focus on the primary action.

@bmingles bmingles merged commit ffb7ada into deephaven:main Feb 27, 2023
@bmingles bmingles deleted the 1109-is-confirm-danger-prop branch February 27, 2023 22:16
@github-actions github-actions bot locked and limited conversation to collaborators Feb 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BasicModal - isConfirmDanger prop
3 participants