-
Notifications
You must be signed in to change notification settings - Fork 25
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(UserFeedback): Add user feedback cards #409
base: main
Are you sure you want to change the base?
Conversation
Preview: https://chatbot-pr-chatbot-409.surge.sh A11y report: https://chatbot-pr-chatbot-409-a11y.surge.sh |
9cd0f62
to
ca8fc57
Compare
ca8fc57
to
22cfcd3
Compare
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.
Sorry in advance for the wall of musing 😅 but, curious for your thoughts. And like I said, these can also just be thoughts for future follow ups. Doesn't have to hold back this pr!
packages/module/patternfly-docs/content/extensions/chatbot/examples/Messages/Messages.md
Outdated
Show resolved
Hide resolved
When a user selects a positive or negative [message action](#message-actions), you can choose to display a feedback card under the message. These cards can be displayed to gather additional feedback and acknowledge a response. The card will be focused on load by default, but this can be customized by setting the `focusOnLoad` prop to false. This prop is set to false in many of these examples so that focus is unaffected, but you will want to leave this on in a standard context. | ||
|
||
Cards can be closed manually via the close button or be configured to time out (see [below](/patternfly-ai/chatbot/messages#message-feedback-response-with-timeouts)). These examples demonstrate the full feedback flow we recommend (namely, submitting additional feedback and seeing the thank you card), just the feedback card, the feedback card without a text input, the feedback card without a close button, just the thank-you card, and the thank-you card without a close button. Additional props are available for further customization. | ||
|
||
The full feedback flow example also demonstrates how to handle focus appropriately for accessibility. The card will be focused when it appears in the DOM. When the card closes, place the focus back on the launching button. You can also add `aria-expanded` and `aria-controls` attributes to the feedback buttons to provide additional context on what the button controls. | ||
|
||
It is also important to announce when new content appears onscreen for accessibility purposes. If you set `isLiveRegion` to true on `<Message>`, it will make appropriate announcements for you when the feedback card appears. |
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.
When a user selects a positive or negative [message action](#message-actions), you can choose to display a feedback card under the message. These cards can be displayed to gather additional feedback and acknowledge a response. The card will be focused on load by default, but this can be customized by setting the `focusOnLoad` prop to false. This prop is set to false in many of these examples so that focus is unaffected, but you will want to leave this on in a standard context. | |
Cards can be closed manually via the close button or be configured to time out (see [below](/patternfly-ai/chatbot/messages#message-feedback-response-with-timeouts)). These examples demonstrate the full feedback flow we recommend (namely, submitting additional feedback and seeing the thank you card), just the feedback card, the feedback card without a text input, the feedback card without a close button, just the thank-you card, and the thank-you card without a close button. Additional props are available for further customization. | |
The full feedback flow example also demonstrates how to handle focus appropriately for accessibility. The card will be focused when it appears in the DOM. When the card closes, place the focus back on the launching button. You can also add `aria-expanded` and `aria-controls` attributes to the feedback buttons to provide additional context on what the button controls. | |
It is also important to announce when new content appears onscreen for accessibility purposes. If you set `isLiveRegion` to true on `<Message>`, it will make appropriate announcements for you when the feedback card appears. | |
When a user selects a positive or negative [message action](#message-actions), you can display a message feedback card that acknowledges their response and provides space for additional written feedback. These cards can be manually dismissed via the close button or be [configured to time out automatically](/patternfly-ai/chatbot/messages#message-feedback-response-with-timeouts). | |
The message feedback card will immediately receive focus by default, but you remove this behavior by passing `focusOnLoad: false` to the `<Message>` (as shown in the following examples). For better usability, you should generally keep the default focus behavior. | |
The following examples demonstrate: | |
- A full feedback flow, which accepts written feedback submission and displays the thank you card. | |
- A basic card. | |
- A basic card without text input. | |
- A card without a close button. | |
- Thank-you cards, with and without a close button. | |
The full feedback flow example also demonstrates how to handle focus appropriately for accessibility. The card will be focused when it appears in the DOM. When the card closes, place the focus back on the launching button. You can also add `aria-expanded` and `aria-controls` attributes to the feedback buttons to provide additional context on what the button controls. | |
It is also important to announce when new content appears onscreen for accessibility purposes. If you set `isLiveRegion` to true on `<Message>`, it will make appropriate announcements for you when the feedback card appears. |
I'm wondering if the full feedback flow (and the related content at the end of the example) would be better under the demos tab, since it's more interactive (like this https://www.patternfly.org/components/button/react-demos)? This would also help us cut down on the amount of text within this one set of examples.
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.
Thinking out loud more--what's the benefit to having the examples broken apart into these different variations, like having "with" and "without close button" versions of the same feature? I'm just thinking that it could help cut the page length down if we were able to utilize checkboxes to alter smaller parts of examples (like this https://www.patternfly.org/components/code-editor#basic). For example here, having checkboxes for "hide text area" and "hide close button", so that we could have every option within one visual example.
But this is just from a content pov & me worrying about the examples getting too long/difficult to parse through (a complaint we've had in the past for other components). If it's more complicated to implement things like this, I wouldn't want to hold back feature releases due to prettier docs. If there's a functional reason to structure it like we have been for devs, then by all means I defer to that also!
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 see why not. I can work on this on Monday when I'm back. I did merge the text updates and update the links.
packages/module/patternfly-docs/content/extensions/chatbot/examples/Messages/Messages.md
Outdated
Show resolved
Hide resolved
packages/module/patternfly-docs/content/extensions/chatbot/examples/Messages/Messages.md
Outdated
Show resolved
Hide resolved
…mples/Messages/Messages.md Co-authored-by: Erin Donehoo <[email protected]>
…mples/Messages/Messages.md Co-authored-by: Erin Donehoo <[email protected]>
…mples/Messages/Messages.md Co-authored-by: Erin Donehoo <[email protected]>
…mples/Messages/Messages.md Co-authored-by: Erin Donehoo <[email protected]>
9674dbe
to
b682d17
Compare
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 is more from testing the docs out:
- The feedback buttons should have some sort of aria to convey whether they're selected/chosen/etc or not.
aria-expanded
would probably be fine considering they control the rendering of other content. - Since the tooltips of the feedback buttons has the same content as the button's aria-label, we should prevent the tooltip from describing the button, otherwise there could be duplicate announcement. The
aria="none"
prop on Tooltip should prevent it. - This could be a followup, but maybe providing a more unique aria-label on the close buttons of each feedback card? Nothing too verbose, but maybe "Close feedback for message received at [time]"?
- For the list of labels, the aria-label should be more descriptive of what the list is/contains. If it's a sort of "quick feedback" thing, something like "Quick feedback for message received at [time]" or something along those lines
- The textbox aria-label could also include a similar update as the previous 2 bullets, e.g. make it a little more unique with something like "Provide additional feedback for message received at [time]".
- For the feedback card without a close button, we'd want to remove the "optional" badge since the only way to get rid of the feedback card is to submit feedback. Unless the intent is that there would be a "cancel" action alongside the Submit button or similar
- For the timeout variation, we probably shouldn't have a timeout on a feedback card that can still be filled out, especially since it's such a short timeout and there may be no visual indication. Even though the timeout is prevented when hover or focus is on the card, it could still be problematic since it's content that can be filled out and possibly lost. Having a timeout on the "Thank you" card should be fine, but we'd also want to include verbiage about not setting the timeout below a certain threshold -- this may require some further research, though we could also ping design about whether using the same timeout lengths as tooltip/popover would be okay.
- Minor thing, but looks like there's a comma being rendered in the timeout example between the chatbot messages:
Ran had asked for a few things here:
I tried to support these and make this as configurable as possible. That said, I'm wondering if this may be too configurable and therefore annoying. Do want to add a plug-and-play easy-mode that just advances from the form to the thank you by itself, and also allow this for power users?
I tried to build in accessibility features, but I'm unsure if this goes far enough.
I defaulted the textarea to be opt-in. Do we want this opt-out? Let me know. I made close buttons, etc. fully configurable, and copied a lot of behavior from alerts. (Thanks to whoever wrote that awesome code and great tests.)
Docs: