-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Fix API Key flyout double submit #167468
Merged
SiddharthMantri
merged 18 commits into
elastic:main
from
SiddharthMantri:bug-fix-api-key-tabbing
Oct 10, 2023
Merged
Fix API Key flyout double submit #167468
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
096e58e
experiment with eui focus trap
SiddharthMantri df84d09
add prop to manage custom onBlur behavior
SiddharthMantri b78ddd1
Merge branch 'main' into bug-fix-api-key-tabbing
SiddharthMantri 21ac5ce
Merge branch 'main' into bug-fix-api-key-tabbing
SiddharthMantri b738ab3
hacky fix for multiple enters on api key name input
SiddharthMantri 7ad56a1
Merge branch 'main' into bug-fix-api-key-tabbing
SiddharthMantri bedbf64
use form onSubmit and isSubmitting instead of onKeyDown
SiddharthMantri d21c2cc
Merge branch 'main' into bug-fix-api-key-tabbing
SiddharthMantri 8d471db
Merge branch 'main' into bug-fix-api-key-tabbing
SiddharthMantri 400a957
remove unused code
SiddharthMantri 1d6ec5e
remove whitespace
SiddharthMantri a4603e4
Merge branch 'main' into bug-fix-api-key-tabbing
SiddharthMantri 60a33d1
revert formfield changes and invert form control and EUI flyout
SiddharthMantri a602ec7
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine 61405ae
Fix form wrapper css for Flyout
SiddharthMantri 92c35ec
Merge branch 'main' into bug-fix-api-key-tabbing
SiddharthMantri cc0901c
Merge branch 'main' into bug-fix-api-key-tabbing
SiddharthMantri 0007558
remove unused FormFlyout component
SiddharthMantri File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,6 @@ import { | |
EuiFlyoutBody, | ||
EuiFlyoutFooter, | ||
EuiFlyoutHeader, | ||
EuiPortal, | ||
EuiTitle, | ||
} from '@elastic/eui'; | ||
import type { FunctionComponent, RefObject } from 'react'; | ||
|
@@ -59,46 +58,44 @@ export const FormFlyout: FunctionComponent<FormFlyoutProps> = ({ | |
const titleId = useHtmlId('formFlyout', 'title'); | ||
|
||
return ( | ||
<EuiPortal> | ||
<EuiFlyout onClose={onCancel} aria-labelledby={titleId} {...rest}> | ||
<EuiFlyoutHeader hasBorder> | ||
<EuiTitle size="m"> | ||
<h2 id={titleId}>{title}</h2> | ||
</EuiTitle> | ||
</EuiFlyoutHeader> | ||
<EuiFlyoutBody>{children}</EuiFlyoutBody> | ||
<EuiFlyoutFooter> | ||
<EuiFlexGroup justifyContent="spaceBetween"> | ||
<EuiFlyout onClose={onCancel} aria-labelledby={titleId} {...rest}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these changes necessary? Does |
||
<EuiFlyoutHeader hasBorder> | ||
<EuiTitle size="m"> | ||
<h2 id={titleId}>{title}</h2> | ||
</EuiTitle> | ||
</EuiFlyoutHeader> | ||
<EuiFlyoutBody>{children}</EuiFlyoutBody> | ||
<EuiFlyoutFooter> | ||
<EuiFlexGroup justifyContent="spaceBetween"> | ||
<EuiFlexItem grow={false}> | ||
<EuiButtonEmpty | ||
data-test-subj="formFlyoutCancelButton" | ||
flush="right" | ||
isDisabled={isLoading} | ||
onClick={onCancel} | ||
> | ||
<FormattedMessage | ||
id="xpack.security.formFlyout.cancelButton" | ||
defaultMessage="Cancel" | ||
/> | ||
</EuiButtonEmpty> | ||
</EuiFlexItem> | ||
{!isSubmitButtonHidden && ( | ||
<EuiFlexItem grow={false}> | ||
<EuiButtonEmpty | ||
data-test-subj="formFlyoutCancelButton" | ||
flush="right" | ||
isDisabled={isLoading} | ||
onClick={onCancel} | ||
<EuiButton | ||
data-test-subj="formFlyoutSubmitButton" | ||
isLoading={isLoading} | ||
isDisabled={isDisabled} | ||
color={submitButtonColor} | ||
fill | ||
onClick={onSubmit} | ||
> | ||
<FormattedMessage | ||
id="xpack.security.formFlyout.cancelButton" | ||
defaultMessage="Cancel" | ||
/> | ||
</EuiButtonEmpty> | ||
{submitButtonText} | ||
</EuiButton> | ||
</EuiFlexItem> | ||
{!isSubmitButtonHidden && ( | ||
<EuiFlexItem grow={false}> | ||
<EuiButton | ||
data-test-subj="formFlyoutSubmitButton" | ||
isLoading={isLoading} | ||
isDisabled={isDisabled} | ||
color={submitButtonColor} | ||
fill | ||
onClick={onSubmit} | ||
> | ||
{submitButtonText} | ||
</EuiButton> | ||
</EuiFlexItem> | ||
)} | ||
</EuiFlexGroup> | ||
</EuiFlyoutFooter> | ||
</EuiFlyout> | ||
</EuiPortal> | ||
)} | ||
</EuiFlexGroup> | ||
</EuiFlyoutFooter> | ||
</EuiFlyout> | ||
); | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's another consumer of the FormFlyout component anymore so it would be best to remove it from the codebase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thomheymann Thanks for confirming, Thom!
@SiddharthMantri Looks like it's used in the 7.17 branch. So if we have the double submit issue in 7.17 and it's worth fixing, we can open a PR against that branch with the changes to
FormFlyout
.