-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
CS-115: NPS Banner UI component #17485
Conversation
Size Change: +1.17 kB (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
packages/core/admin/admin/src/components/NpsSurvey/hooks/useNpsSurveySettings.js
Outdated
Show resolved
Hide resolved
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.
Overall i'm not sure about the semantic direction, we've opted for textarea and buttons and then submit it all, do you not think this would be better as a form component overall? and then the rating is a semantically a radio-button? I don't think what you've done is "bad" I just think it can be better 💪🏼 Be interested to hear what the others think – @simotae14, @markkaylor & @gu-stav
@madhurisandbhor if it's easier for you, we can arrange a call to discuss it in more detail
packages/core/admin/admin/src/components/NpsSurvey/tests/index.test.js
Outdated
Show resolved
Hide resolved
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.
Very nice!
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.
Code-wise this is ready to go I think. Good job, @madhurisandbhor! I haven't done any functional QA yet - @Feranchz could you do that?
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.
Hey @madhurisandbhor it looks nice! great work
looks like the close button is not on the square anymore
fixed |
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.
🚀
Please do a squash and merge, well done @madhurisandbhor |
* added survey rating and feedbackbox with submit button * added disabled prop for submit button * reverting disabled prop, not required * review comments updated * button replaced with radio button, wrapped in form * review comments updated * reverted display time * accessiblity changes * styles updated
* added survey rating and feedbackbox with submit button * added disabled prop for submit button * reverting disabled prop, not required * review comments updated * button replaced with radio button, wrapped in form * review comments updated * reverted display time * accessiblity changes * styles updated
* added survey rating and feedbackbox with submit button * added disabled prop for submit button * reverting disabled prop, not required * review comments updated * button replaced with radio button, wrapped in form * review comments updated * reverted display time * accessiblity changes * styles updated
* added survey rating and feedbackbox with submit button * added disabled prop for submit button * reverting disabled prop, not required * review comments updated * button replaced with radio button, wrapped in form * review comments updated * reverted display time * accessiblity changes * styles updated
What does it do?
NPS Banner UI component added with dumb submit button.
Why is it needed?
Issue
How to test it?
If the user checked the “Keep me updated” box when registering to Strapi, then user will see the NPS survey banner with the form to fill out at the bottom of the screen.