-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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(TimePickerSelect): updated props for v11 #9914
feat(TimePickerSelect): updated props for v11 #9914
Conversation
✔️ Deploy Preview for carbon-react-next ready! 🔨 Explore the source changes: 7edaf41 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/6179f6f5190cef0007f8272d 😎 Browse the preview: https://deploy-preview-9914--carbon-react-next.netlify.app |
✔️ Deploy Preview for carbon-elements ready! 🔨 Explore the source changes: 7edaf41 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/6179f6f52e83dd0007335ad2 😎 Browse the preview: https://deploy-preview-9914--carbon-elements.netlify.app |
✔️ Deploy Preview for carbon-components-react ready! 🔨 Explore the source changes: 7edaf41 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/6179f6f500e7b10007063332 😎 Browse the preview: https://deploy-preview-9914--carbon-components-react.netlify.app |
packages/react/src/components/TimePickerSelect/TimePickerSelect.js
Outdated
Show resolved
Hide resolved
…t.js Co-authored-by: Abbey Hart <abbeyhrt@gmail.com>
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 looks good!
Do we need to change the labelClasses
block w/ visually-hidden
to always be true since the hideLabel
default has been removed? https://github.com/carbon-design-system/carbon/pull/9914/files#diff-a67cbb553a741d3befb1f3b64288b2c0acf8e9f033976bf1f2bd5a7553138ad8R97-R100
packages/react/src/components/TimePickerSelect/TimePickerSelect.js
Outdated
Show resolved
Hide resolved
…t.js Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com>
I'm going to do that with this PR. |
packages/react/src/components/TimePickerSelect/TimePickerSelect.js
Outdated
Show resolved
Hide resolved
packages/react/src/components/TimePickerSelect/TimePickerSelect.js
Outdated
Show resolved
Hide resolved
packages/react/src/components/TimePickerSelect/TimePickerSelect.js
Outdated
Show resolved
Hide resolved
…t.js Co-authored-by: Josh Black <josh@josh.black>
…into feat/timepickerselect
…t.js Co-authored-by: Josh Black <josh@josh.black>
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.
🎉
### Related Ticket(s) carbon-design-system#9567 ### Description This updates the React masthead logo URL to use `https`. ### Changelog **Changed** - React masthead logo URL changed to use `https` <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "test: e2e": Codesandbox examples and e2e integration tests --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
REF #9535 #9624
Refactoring
TimePickerSelect
with prop changes in preparation for migration to a functional component.Storybook
iconDescription
from "Select" in theTimePicker
story.aria-label
knob to "Select" in theTimePicker
story.TimePickerSelect
iconDescription
.inline
prop....other
->...rest
ariaLabel
prop.Testing
TimePicker
tests are passing.TimePicker
does it's thing in Storybook, especially as it pertains toTimePickerSelect
.