-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix(Dropdown): Made the titleText required property #17854
Fix(Dropdown): Made the titleText required property #17854
Conversation
All contributors have signed the DCO. |
I have read the DCO document and I hereby sign the DCO. |
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
27025c1
to
75ab61b
Compare
recheck |
75ab61b
to
d644eb3
Compare
d644eb3
to
1f5c1e2
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.
I think this should be required even though it's conditionally rendered.
@tay1orjones updated the code to make the titleText property required instead of optional. |
8a86889
to
5afd492
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.
LGTM!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17854 +/- ##
=======================================
Coverage 80.79% 80.79%
=======================================
Files 406 406
Lines 14041 14041
Branches 4395 4395
=======================================
Hits 11344 11344
+ Misses 2531 2530 -1
- Partials 166 167 +1 ☔ View full report in Codecov by Sentry. |
380dbcf
to
fb213a0
Compare
Head branch was pushed to by a user without write access
bcae1a1
to
d54f8c4
Compare
e4386e6
Closes #17843
This PR makes the
titleText
an optional property in the propTypes, since we are only showingtitleText
conditionally.Changelog
Changed
titleText
optional in propTypes.Testing / Reviewing
titleText
property from in "packages/react/src/components/Dropdown/Dropdown.stories.js"cd packages/react
and runyarn storybook