Skip to content
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(RadioButton): refactor radio btn and btn group to functional components #9940

Merged
merged 14 commits into from
Oct 27, 2021

Conversation

jnm2377
Copy link
Contributor

@jnm2377 jnm2377 commented Oct 25, 2021

Ref #9712

Changelog

  • refactor RadioButton
  • refactor RadioButtonGroup
  • adds tests in /next folder (these tests were just copied over from v10, nothing changed).

Testing / Reviewing

@netlify
Copy link

netlify bot commented Oct 25, 2021

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

🔨 Explore the source changes: 9a71689

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

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

@netlify
Copy link

netlify bot commented Oct 25, 2021

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

🔨 Explore the source changes: 9a71689

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

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

@netlify
Copy link

netlify bot commented Oct 25, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 9a71689

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

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

@jnm2377 jnm2377 marked this pull request as ready for review October 26, 2021 16:16
@jnm2377 jnm2377 requested a review from a team as a code owner October 26, 2021 16:16
jnm2377 and others added 6 commits October 26, 2021 19:52
…nGroup.js

Co-authored-by: Josh Black <josh@josh.black>
Co-authored-by: Scott Strubberg <sstrubberg@protonmail.com>
Co-authored-by: Scott Strubberg <sstrubberg@protonmail.com>
@jnm2377 jnm2377 merged commit 711a35f into carbon-design-system:main Oct 27, 2021
kennylam pushed a commit to kennylam/carbon that referenced this pull request Jul 30, 2024
…ign-system#10080)

### Related Ticket(s)

Closes carbon-design-system#9940 

### Description

This PR takes a new approach, and offers some advantages of carbon-design-system#9941.

While the other PR aims to separate class definitions from their registration in the browser, and should work well for teams or projects that wish to _extend_ C4IBM components, it does nothing to solve the problem for teams that are _double-consuming_ C4IBM components. For example, consider the widget team:

- They are building widgets using the react-wrapper version of the components
- When the react component imports the element, it brings along its `customElements.define()` method
- The widget is placed on a page that is already using the C4IBM CDN links, which _also_ bring along the `customElements.define()` method.
- Both the widget script and the CDN link are rolled up into separate scripts and whichever one runs second will encounter the unexpected error, not know how to handle that problem, and halt execution of the script, likely leaving large pieces of setup functionality unprocessed

This new approach offers a method for elements to _attempt_ to define themselves, but to gracefully handle the failure and to continue executing the remainder of the running script. The new decorator is a simple copy/paste from the LitElement `@customElement` decorator, with the `customElements.define` functions wrapped in a try/catch block.

As of right now, this only impacts one component and is simply a proof-of-concept.

### Changelog

**New**

- Adds a `@carbonElement()` decorator to the carbon-web-components package

**Changed**

- Edit the carbon-web-components package's accordion-item component to use @CarbonElement
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants