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.

Sign up for GitHub

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

Jump to bottom

feat!: support CSS-in-JS solutions #226

Merged
merged 2 commits into from
Apr 28, 2022
Merged

feat!: support CSS-in-JS solutions #226

merged 2 commits into from
Apr 28, 2022

Conversation

karolisgrinkevicius
Copy link
Contributor

Description

Make component compatible with emotion and styled-components providing the className for parent element.

Motivation

Previously this component wasn't compatible neither with emotion css in js nor styled-components due to the class name being set for child element.

Notes

Duplicated by #209

@karolisgrinkevicius karolisgrinkevicius mentioned this pull request Apr 3, 2020
Closed
tjallingt
tjallingt approved these changes Apr 3, 2020
View reviewed changes
Copy link
Owner

@tjallingt tjallingt left a comment

Choose a reason for hiding this comment

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

Thanks for resubmitting! one small nit and then we can land this.

src/Youtube.test.js Outdated Show resolved Hide resolved
@tjallingt tjallingt added Type: Breaking Change This change will require a new major version Status: Changes Requested labels Apr 3, 2020
@ruisaraiva19
Copy link
Collaborator

As this is a breaking change and we are using conventional commits/semantic releases, @karolisgrinkevicius please add a breaking change footer to your commit as well.
Here is a link for reference: https://www.conventionalcommits.org/en/v1.0.0/#commit-message-with-description-and-breaking-change-footer

@karolisgrinkevicius
Copy link
Contributor Author

As this is a breaking change and we are using conventional commits/semantic releases, @karolisgrinkevicius please add a breaking change footer to your commit as well.
Here is a link for reference: https://www.conventionalcommits.org/en/v1.0.0/#commit-message-with-description-and-breaking-change-footer

Thanks @ruisaraiva19. Already fixed by adding the exclamation mark in the commit message.

@karolisgrinkevicius
Copy link
Contributor Author

karolisgrinkevicius commented Apr 11, 2020 •
edited
Loading

@ruisaraiva19 any news on merging this PR?

@tjallingt
Copy link
Owner

I tend to sit on breaking changes for a bit, so that it is maybe possible to bundle them with other breaking changes.
Also there where some issues with the engines field so i was waiting to see a resolution there so these things don't start interfering.
I'll try to get this merged asap, feel free to bump whenever though

@ruisaraiva19 ruisaraiva19 changed the title Make component compatible with styled-components providing the className for parent element feat: support styled-components Apr 14, 2022
@ruisaraiva19
Copy link
Collaborator

@karolisgrinkevicius can you sync your branch with the latest from canary?

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 15, 2022 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 59fe216:

Sandbox Source
react-youtube-example Configuration

@karolisgrinkevicius
Copy link
Contributor Author

@ruisaraiva19 thanks, rebased on canary branch already.

ruisaraiva19
ruisaraiva19 requested changes Apr 17, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
packages/react-youtube/src/YouTube.tsx Outdated Show resolved Hide resolved
packages/react-youtube/src/YouTube.tsx Outdated Show resolved Hide resolved
packages/react-youtube/src/YouTube.tsx Outdated Show resolved Hide resolved
packages/react-youtube/src/YouTube.tsx Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
packages/react-youtube/src/YouTube.tsx Outdated Show resolved Hide resolved
ruisaraiva19
ruisaraiva19 reviewed Apr 19, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
@ruisaraiva19
Copy link
Collaborator

@karolisgrinkevicius can you sync with the latest changes from canary?

@karolisgrinkevicius-home24
Copy link

@karolisgrinkevicius can you sync with the latest changes from canary?

@ruisaraiva19 synced 🔄 ✅

Repository owner deleted a comment from karolisgrinkevicius-home24 Apr 27, 2022
Repository owner deleted a comment from karolisgrinkevicius-home24 Apr 27, 2022
ruisaraiva19
ruisaraiva19 requested changes Apr 27, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
packages/react-youtube/src/YouTube.tsx Outdated Show resolved Hide resolved
packages/react-youtube/src/YouTube.tsx Outdated Show resolved Hide resolved
packages/react-youtube/src/YouTube.tsx Outdated Show resolved Hide resolved
packages/react-youtube/src/YouTube.tsx Outdated Show resolved Hide resolved
@ruisaraiva19 ruisaraiva19 changed the title feat: support styled-components feat!: support styled-components Apr 27, 2022
@karolisgrinkevicius
Copy link
Contributor Author

@ruisaraiva19 please feel free to merge it and then release it if you are fine with the recent changes.

@ruisaraiva19 ruisaraiva19 changed the title feat!: support styled-components feat!: support CSS-in-JS solutions Apr 28, 2022
@ruisaraiva19 ruisaraiva19 merged commit 5770838 into tjallingt:canary Apr 28, 2022
ruisaraiva19 pushed a commit that referenced this pull request Apr 28, 2022
@karolisgrinkevicius @ruisaraiva19
feat: support CSS-in-JS solutions (#226)
654418d 
BREAKING CHANGE: the `containerClassName` prop was renamed to `className`,
the `containerStyle` prop was renamed to `style`,
and the iframe class is now passed via the `iframeClassName` prop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karolisgrinkevicius-home24 karolisgrinkevicius-home24 karolisgrinkevicius-home24 approved these changes

@ruisaraiva19 ruisaraiva19 ruisaraiva19 approved these changes

@tjallingt tjallingt tjallingt approved these changes

Assignees
No one assigned
Labels
Status: Ready To Review Type: Breaking Change This change will require a new major version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@karolisgrinkevicius @ruisaraiva19 @tjallingt @karolisgrinkevicius-home24