-
Notifications
You must be signed in to change notification settings - Fork 24
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
Feature/add mandatory asterisk #2871
Feature/add mandatory asterisk #2871
Conversation
@anehx do you want to take a look at this one as well? |
packages/form/tests/integration/components/cf-field/label-test.js
Outdated
Show resolved
Hide resolved
packages/form/tests/integration/components/cf-field/label-test.js
Outdated
Show resolved
Hide resolved
packages/form/tests/integration/components/cf-field/label-test.js
Outdated
Show resolved
Hide resolved
Also what I forgot in my review: We test if it works when that option is passed directly to the component which is very unlikely to happen but not whether the actual configuration of that feature using |
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.
Some really minor changes are still needed but in general this looks great!
packages/form/tests/integration/components/cf-field/label-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.
Perfect! 💯
This can be merged (and squashed) as soon as the linter is happy.
Description
added mandatory asterisk configuration option for labels of form questions