Skip to content

feat(TimePickerSelect): class -> functional#9923

Merged
sstrubberg merged 8 commits into
carbon-design-system:mainfrom
sstrubberg:feat/timepickerselect-functional
Oct 30, 2021
Merged

feat(TimePickerSelect): class -> functional#9923
sstrubberg merged 8 commits into
carbon-design-system:mainfrom
sstrubberg:feat/timepickerselect-functional

Conversation

@sstrubberg

@sstrubberg sstrubberg commented Oct 21, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

REF #9712

Refactored TimePickerSelect into a functional components.

Changed

Removed hideLabel prop and label logic.
Removed deprecated iconDescription.
Added aria-label to the select element
Added aria-hidden to the icon.

Testing / Reviewing

  1. Pull down locally
  2. Go to packages/react
  3. Run yarn start:v11 to run old storybook with v11 components.

Questions

  • Should I have just removed the hideLabel prop and kept the label logic? In other words, do we need <label> for anything?

@netlify

netlify Bot commented Oct 21, 2021 •
edited
Loading

Copy link
Copy Markdown

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: 448a8f1

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/617d322adc50bc000739e8af

😎 Browse the preview: https://deploy-preview-9923--carbon-react-next.netlify.app

@netlify

netlify Bot commented Oct 21, 2021 •
edited
Loading

Copy link
Copy Markdown

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 448a8f1

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/617d322a4992c9000785ac00

😎 Browse the preview: https://deploy-preview-9923--carbon-components-react.netlify.app

@netlify

netlify Bot commented Oct 21, 2021 •
edited
Loading

Copy link
Copy Markdown

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 448a8f1

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/617d322a8a8eb50008027c46

😎 Browse the preview: https://deploy-preview-9923--carbon-elements.netlify.app

@sstrubberg sstrubberg mentioned this pull request Oct 25, 2021
Merged
@sstrubberg sstrubberg marked this pull request as ready for review October 26, 2021 21:13
@sstrubberg sstrubberg requested a review from a team as a code owner October 26, 2021 21:13
@joshblack joshblack assigned abbeyhrt and unassigned abbeyhrt Oct 27, 2021
@joshblack joshblack requested a review from abbeyhrt October 27, 2021 21:05
jnm2377
jnm2377 reviewed Oct 28, 2021
View reviewed changes
Comment thread packages/react/src/components/TimePickerSelect/next/TimePickerSelect.js Outdated
@jnm2377

jnm2377 commented Oct 28, 2021

Copy link
Copy Markdown
Contributor

Also, are you adding tests to the /next folder?

@sstrubberg

sstrubberg commented Oct 28, 2021

Copy link
Copy Markdown
Member Author

Also, are you adding tests to the /next folder?

derp.. thx for the reminder.

@sstrubberg

sstrubberg commented Oct 29, 2021

Copy link
Copy Markdown
Member Author

@jnm2377 now i remember. TimePickerSelect currently has no tests.

@sstrubberg sstrubberg requested a review from jnm2377 October 29, 2021 17:37
@sstrubberg sstrubberg enabled auto-merge (squash) October 29, 2021 21:06
@sstrubberg sstrubberg merged commit 780f6df into carbon-design-system:main Oct 30, 2021
kennylam pushed a commit to kennylam/carbon that referenced this pull request Jul 30, 2024
@annawen1
fix(styles): add missing styles for breadcrumb and modal (carbon-desi…
c92bdae 
…gn-system#9942)

### Related Ticket(s)

[breadcrumb]: / separator not showing on v1.27.0 carbon-design-system#9936
Modal not displaying with @carbon/web-components 1.23.0 carbon-design-system#9923

### Description

Some styles have not been extending from `carbon-components` for `bx-modal` and `bx-breadcrumb` components. Will need to dig further to see why they aren't showing, but in the mean time we can add those styles to ours.

### Changelog

**Changed**

- added styles from `carbon-components` that are not showing for `bx-modal` and `bx-breadcrumb`

<!-- 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) -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@joshblack joshblack joshblack approved these changes

@abbeyhrt abbeyhrt abbeyhrt approved these changes

@jnm2377 jnm2377 jnm2377 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sstrubberg @jnm2377 @joshblack @abbeyhrt @tay1orjones @dakahn