-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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: add alerts for redesigned contract interaction confirmation #25174
Changes from all commits
327cfe7
a215a7b
3b2a14a
be66b21
616fb54
8b30fcd
c97e962
4dd53bf
1d4f81b
27b0e91
666871a
23de612
7c1ec03
a8f9dd5
37f6c86
161691b
210d128
4cd607a
5d5b5ab
ad2fce0
1d925c9
600d31b
3d87ef3
c9025ec
ffbd54e
27f9cc6
abee921
e78cf18
8c0eeb5
09a1cc9
05d51c3
13cec31
6c54f9f
395e655
6cc412a
67c94f2
325991f
cb23cfb
19533e5
5278ff5
e58702c
9f43cbb
dad1bfa
f07eb63
04229ba
500e1ba
ff442cf
2c44cba
6729dd4
0bd3d0c
956e494
0dde6d0
3441fe1
675d90a
e2138dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,11 +49,11 @@ export const ConfirmInfoAlertRow = ({ | |
|
||
const [alertModalVisible, setAlertModalVisible] = useState<boolean>(false); | ||
|
||
const handleCloseModal = () => { | ||
const handleModalClose = () => { | ||
setAlertModalVisible(false); | ||
}; | ||
|
||
const handleOpenModal = () => { | ||
const handleInlineAlertClick = () => { | ||
setAlertModalVisible(true); | ||
}; | ||
|
||
|
@@ -66,18 +66,21 @@ export const ConfirmInfoAlertRow = ({ | |
|
||
const inlineAlert = hasFieldAlert ? ( | ||
<Box marginLeft={1}> | ||
<InlineAlert onClick={handleOpenModal} severity={selectedAlertSeverity} /> | ||
<InlineAlert | ||
onClick={handleInlineAlertClick} | ||
severity={selectedAlertSeverity} | ||
/> | ||
</Box> | ||
) : null; | ||
|
||
return ( | ||
<> | ||
{alertModalVisible && ( | ||
<MultipleAlertModal | ||
alertKey={alertKey} | ||
alertKey={fieldAlerts[0].key} | ||
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. I am bit confused here, is it ok to hardcode key to always alert at index 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. It is safe, since the modal isn't visible unless the alert is clicked, which is only shown if there are field alerts. 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. possibly renaming 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. I like that, but perhaps we could do that in a separate PR to limit the scope of this big one? |
||
ownerId={ownerId} | ||
onFinalAcknowledgeClick={handleCloseModal} | ||
onClose={handleCloseModal} | ||
onFinalAcknowledgeClick={handleModalClose} | ||
onClose={handleModalClose} | ||
/> | ||
)} | ||
<ConfirmInfoRow {...confirmInfoRowProps} labelChildren={inlineAlert} /> | ||
|
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.
May be we move this to hook
useAlerts
. Hook can be passedselectedAlertKey
.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.
This was simply to make it more readable, plus handle if the alert key isn't found instead of throwing.