-
Notifications
You must be signed in to change notification settings - Fork 164
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: Blue dialog code snippet #2923
base: main
Are you sure you want to change the base?
Changes from all commits
4de26b6
e3ab331
23677fb
e78ac03
e099eaa
8436467
3c3014f
eb3e450
7d474df
793d280
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
import React from 'react'; | ||
|
||
import { Box } from '~components'; | ||
|
||
import Bluedialog from './dialog'; | ||
|
||
export default function BluedialogPage() { | ||
const [showDialog, setShowDialog] = React.useState(true); | ||
const submitData = (data: any) => { | ||
console.log(data); // submit the form with these values to determine next action | ||
}; | ||
const skipDialog = () => { | ||
setShowDialog(false); | ||
}; | ||
return <Box padding="xxl">{showDialog && <Bluedialog onSubmit={submitData} onSkip={skipDialog} />}</Box>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
import React, { useEffect, useRef } from 'react'; | ||
|
||
import { Box, Button, Checkbox, Form, FormField, SpaceBetween, Textarea } from '~components'; | ||
|
||
import styles from './styles.scss'; | ||
|
||
export default function Bluedialog({ onSubmit, onSkip }: any) { | ||
const dialogRef = useRef<HTMLDivElement>(null); | ||
const triggerRef = useRef<HTMLElement | null>(null); | ||
// Inputs (these following can be clubbed into one state object) | ||
const [feedbackOptions, setFeedbackOptions] = React.useState({ | ||
harmful: false, | ||
incomplete: false, | ||
inaccurate: false, | ||
other: false, | ||
}); | ||
const [feedbackText, setFeedbackText] = React.useState(''); | ||
|
||
useEffect(() => { | ||
// Store the element that had focus before dialog opened | ||
triggerRef.current = document.activeElement as HTMLElement; | ||
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. Would it make more sense to handle the focus management from the outside using the onSkip? Otherwise there are corner cases like if the trigger disappears after opening the dialog etc. that are not covered by this approach. Or, if the dialog opens after an async transition or automatically - the last focused element might not be related to the dialog at all. |
||
|
||
// Focus the dialog container when it opens | ||
if (dialogRef.current) { | ||
dialogRef.current.focus(); | ||
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. The dialog ref is not assigned anywhere so this code does nothing. |
||
} | ||
// Cleanup: Return focus to the trigger element when dialog closes | ||
return () => { | ||
if (triggerRef.current) { | ||
triggerRef.current.focus(); | ||
} | ||
}; | ||
}, []); | ||
|
||
function submitData() { | ||
onSubmit({ feedbackOptions, feedbackText }); | ||
} | ||
Comment on lines
+37
to
+39
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. Maybe we could add validation here to ensure one of the checkboxes are selected before submission 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 was not aware of this use case. Hence avoided it to leave it on the user's choice. I will implement it in the new revision. 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. Actually, I am not sure how we want to handle it. This needs to be discussed if there can be a use case where we don't want to select any option, but just want to submit the 'Tell us more' section. Will implement it after discussion. 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. But "Tell us more" is optional. I believe at least one of the boxes should be checked but we can discuss it. |
||
function skipDialog() { | ||
onSkip(); // can be used by the parent component to dismiss/ hide this dialog box | ||
} | ||
|
||
return ( | ||
<div | ||
className={styles['blue-dialog-box']} | ||
role={'dialog'} | ||
aria-labelledby="feedback dialog" | ||
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. feedback-dialog is not defined anywhere, could you replace this with aria-label? |
||
aria-modal="false" // Maintains natural focus flow since it's an inline dialog | ||
tabIndex={-1} // This allows the dialog to receive focus | ||
> | ||
<Form> | ||
<Box padding={'l'}> | ||
<div className={styles['blue-dialog-box__close']}> | ||
<Button iconName="close" variant="icon" onClick={skipDialog} aria-label="Close dialog" /> | ||
</div> | ||
<SpaceBetween direction="vertical" size="l"> | ||
<FormField label="What did you dislike about the response?"> | ||
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. We still need to show a dismiss button 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. Done in the new Revision. Thanks for pointing it out |
||
<SpaceBetween size={'xxl'} direction={'horizontal'}> | ||
<Checkbox | ||
checked={feedbackOptions.harmful} | ||
onChange={({ detail }) => setFeedbackOptions({ ...feedbackOptions, harmful: detail.checked })} | ||
> | ||
Harmful | ||
</Checkbox> | ||
<Checkbox | ||
checked={feedbackOptions.incomplete} | ||
onChange={({ detail }) => setFeedbackOptions({ ...feedbackOptions, incomplete: detail.checked })} | ||
> | ||
Incomplete | ||
</Checkbox> | ||
<Checkbox | ||
checked={feedbackOptions.inaccurate} | ||
onChange={({ detail }) => setFeedbackOptions({ ...feedbackOptions, inaccurate: detail.checked })} | ||
> | ||
Inaccurate | ||
</Checkbox> | ||
<Checkbox | ||
checked={feedbackOptions.other} | ||
onChange={({ detail }) => setFeedbackOptions({ ...feedbackOptions, other: detail.checked })} | ||
> | ||
Other | ||
</Checkbox> | ||
</SpaceBetween> | ||
</FormField> | ||
<FormField label="Tell us more - optional" stretch={true}> | ||
<Textarea | ||
rows={1} | ||
onChange={({ detail }) => setFeedbackText(detail.value)} | ||
value={feedbackText} | ||
placeholder={'Additional feedback'} | ||
/> | ||
</FormField> | ||
</SpaceBetween> | ||
</Box> | ||
|
||
<div className={styles['blue-dialog-box__footer']}> | ||
<Button onClick={submitData} ariaLabel="Submit form"> | ||
Submit | ||
</Button> | ||
</div> | ||
</Form> | ||
</div> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/* | ||
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
@use '~design-tokens' as awsui; | ||
|
||
.blue-dialog-box { | ||
border-block: 1px solid awsui.$color-border-status-info; | ||
border-inline: 1px solid awsui.$color-border-status-info; | ||
background-color: awsui.$color-background-status-info; | ||
border-start-start-radius: 16px; | ||
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. can we use $border-radius-container here? |
||
border-start-end-radius: 16px; | ||
border-end-start-radius: 16px; | ||
border-end-end-radius: 16px; | ||
} | ||
|
||
.blue-dialog-box__close { | ||
float: right; | ||
} | ||
|
||
.blue-dialog-box__footer { | ||
border-block-start: 1px solid awsui.$color-border-divider-default; | ||
padding-block: 10px; | ||
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. can we use design tokens here? |
||
padding-inline: 20px; | ||
display: flex; | ||
flex-direction: row-reverse; | ||
gap: 20px; | ||
} |
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.
Instead of this you can have
selectedFeedbackOption
where you set the value to one of harmful, incomplete, etc. That way you won't need to spread objects during state setting below and it'll be easier to understand. What do you think?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 think this needs to be a multi select, where one or more options can be selected at a time. Hence, I chose to maintain a separate property for each option.
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.
Oh right multi choice makes sense in this case.