-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
chore(feedback): Prefer <FeedbackButton> over useFeedbackForm() #103916
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
Conversation
❌ 46 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
billyvg
left a comment
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.
RSLGTM
| }, | ||
| }} | ||
| > | ||
| {undefined} |
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.
undefined?
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.
ya, need to override the default string :(
there's two places that do this for "👍 👎 " buttons. We could probably make that pair a helper in it's own right and improve this.
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'll follow up with that.
This updates most of the callsites to use
<FeedbackButton>when before it was usinguseFeedbackForm().The element is easier to use than the hook, takes less code and has reasonable defaults that will save you time and extra code. By having most of the examples in the app use the easier element form, people should be copy+pasting the better form, and the LLMS will hopefully prefer it too.
Along the way i noticed a few places that were not translating strings, and/or passing inappropriate stuff to data-aria-label. This is cleaned up. There are more spots in these same files that are missing translations, but i only changed things related to feedback.
I tweaked the FeedbackButton itself so it can better behave as an icon-only button. The Thumbs Up/Down pattern is used twice that i found.
There's a few other places that still call
useFeedbackForm()directly, some can be changed over (bug i didn't find them in the app to test them out) while others are like menu items and stuff that don't need to use a Button.