Skip to content

feat: added types for Accordion, AccordionItem, AccordionSkeleton, SkeletonText#13875

Merged
kodiakhq[bot] merged 12 commits into
carbon-design-system:mainfrom
SunnyJohal:types/accordion-component
Jun 8, 2023
Merged

feat: added types for Accordion, AccordionItem, AccordionSkeleton, SkeletonText#13875
kodiakhq[bot] merged 12 commits into
carbon-design-system:mainfrom
SunnyJohal:types/accordion-component

Conversation

@SunnyJohal

@SunnyJohal SunnyJohal commented May 25, 2023

Copy link
Copy Markdown
Contributor

Closes #13544 #13576

Added type definitions to the Accordion, AccordionItem, AccordionSkeleton, SkeletonTextComponents

Changelog

New

  • Now using context instead of cloning the children within the accordion and accordion item.

Changed

  • Added type definitions to the Accordion, AccordionItem, AccordionSkeleton, SkeletonTextComponents.

Removed

  • Unused IconDescription prop.

Testing / Reviewing

Run unit tests, start up story book and confirm that the accordion is working as before.

@SunnyJohal SunnyJohal requested a review from a team as a code owner May 25, 2023 09:22
@SunnyJohal SunnyJohal mentioned this pull request May 25, 2023
Closed
@SunnyJohal

SunnyJohal commented May 25, 2023

Copy link
Copy Markdown
Contributor Author

@andreancardona I've created a new PR to get around the DCO issue, are you ok to review please. Thanks!

@netlify

netlify Bot commented May 25, 2023 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 339ba55
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/6482346571c56c000855a7c4
😎 Deploy Preview https://deploy-preview-13875--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify

netlify Bot commented May 25, 2023

Copy link
Copy Markdown

✅ Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 58d17d2
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/646f28cab6cb2c00081b1b92
😎 Deploy Preview https://deploy-preview-13875--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@SunnyJohal SunnyJohal changed the title Types/accordion component feat: added types for Accordion, AccordionItem, AccordionSkeleton, SkeletonText May 25, 2023
@netlify

netlify Bot commented May 25, 2023 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 339ba55
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/648234654196d90008002cb8
😎 Deploy Preview https://deploy-preview-13875--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@andreancardona

andreancardona commented May 30, 2023

Copy link
Copy Markdown
Contributor

@SunnyJohal the only thing you need to do here is fix / update the tests so they are no longer failing - please let me know if I can help in any way!

@SunnyJohal

SunnyJohal commented Jun 5, 2023

Copy link
Copy Markdown
Contributor Author

Hi @andreancardona I've made the required changes yesterday but the ci still hasn't completed, is there anything we can do to get this over the line?

@andreancardona

andreancardona commented Jun 6, 2023

Copy link
Copy Markdown
Contributor

Hi @andreancardona I've made the required changes yesterday but the ci still hasn't completed, is there anything we can do to get this over the line?

I just retriggered it - let's hope this passes!

@SunnyJohal SunnyJohal force-pushed the types/accordion-component branch from 879c1dd to 6411e0d Compare June 6, 2023 17:35
@SunnyJohal

SunnyJohal commented Jun 6, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@andreancardona tests are all passing! (I rebased off main) 🎉

andreancardona
andreancardona reviewed Jun 7, 2023
View reviewed changes
Comment thread packages/react/src/components/Accordion/AccordionItem.tsx Outdated
andreancardona
andreancardona approved these changes Jun 7, 2023
View reviewed changes

@andreancardona andreancardona left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

tw15egan
tw15egan reviewed Jun 8, 2023
View reviewed changes
Comment thread packages/react/src/components/Accordion/AccordionItem.tsx Outdated
Comment thread packages/react/src/components/Accordion/AccordionItem.tsx Outdated
Comment thread packages/react/src/components/Accordion/Accordion.tsx Outdated
@SunnyJohal

SunnyJohal commented Jun 8, 2023

Copy link
Copy Markdown
Contributor Author

Hi @tw15egan I've made the requested changes and the CI is passing too, are you ok to approve so that we can merge this in please. Thanks!

tw15egan
tw15egan approved these changes Jun 8, 2023
View reviewed changes

@tw15egan tw15egan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for tackling this! 💪🏻 ✅

@kodiakhq kodiakhq Bot merged commit 8bc7bfc into carbon-design-system:main Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alisonjoseph alisonjoseph Awaiting requested review from alisonjoseph alisonjoseph was automatically assigned from carbon-design-system/developers-system

@tay1orjones tay1orjones Awaiting requested review from tay1orjones tay1orjones was automatically assigned from carbon-design-system/developers-system

2 more reviewers

@tw15egan tw15egan tw15egan approved these changes

@andreancardona andreancardona andreancardona approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status: ready to merge 🎉

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add TypeScript types to Accordion, AccordionItem, AccordionSkeleton

5 participants

@SunnyJohal @andreancardona @tw15egan @alisonjoseph @francinelucca