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

hot-fix(pagination): fix style bug #9346

Merged
merged 5 commits into from
Jul 28, 2021
Merged

hot-fix(pagination): fix style bug #9346

merged 5 commits into from
Jul 28, 2021

Conversation

andreancardona
Copy link
Contributor

@andreancardona andreancardona commented Jul 28, 2021 •
edited
Loading

This is a hot fix reverts this PR: #9138

That did not fix the original issue: #9112

image (5)

@andreancardona andreancardona requested a review from a team as a code owner July 28, 2021 14:29
@netlify
Copy link

netlify bot commented Jul 28, 2021 •
edited
Loading

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

🔨 Explore the source changes: b556652

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

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

@andreancardona andreancardona marked this pull request as draft July 28, 2021 14:29
@andreancardona andreancardona changed the title fix(pagination): fix style bug hot-fix(pagination): fix style bug Jul 28, 2021
@andreancardona andreancardona marked this pull request as ready for review July 28, 2021 14:37
@netlify
Copy link

netlify bot commented Jul 28, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 73c7963

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

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

@netlify
Copy link

netlify bot commented Jul 28, 2021

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

🔨 Explore the source changes: 73c7963

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

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

@netlify
Copy link

netlify bot commented Jul 28, 2021 •
edited
Loading

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: b556652

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

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

@tw15egan
Copy link
Collaborator

This was recently added in via #9138. Is there an alternative fix for #9138 that doesn't break the component in other use-cases?

@andreancardona
Copy link
Contributor Author

andreancardona commented Jul 28, 2021 •
edited
Loading

@tw15egan hmm good question. I tested the available use cases in storybook

@tw15egan
Copy link
Collaborator

It's fine if there isn't, we should just re-open the other issue so we don't forget about it 👍🏻

@netlify
Copy link

netlify bot commented Jul 28, 2021

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

🔨 Explore the source changes: 814e5f7

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

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

@netlify
Copy link

netlify bot commented Jul 28, 2021 •
edited
Loading

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

🔨 Explore the source changes: b556652

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

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

@andreancardona
Copy link
Contributor Author

andreancardona commented Jul 28, 2021 •
edited
Loading

@tw15egan Ooo one other thing. I just asked D.A. to look at this bug again (in windows), and it seems it's still not fixed: #9138

I don't have Windows support. Can you confirm this bug still exists?

Also it seems to be a bug in svelte and vanilla #9112

Am I understanding that correctly? 👀

@tw15egan
Copy link
Collaborator

I don't currently have access to a Windows machine, but if that fix didn't work then lets definitely revert it so that the pagination bug is fixed

tw15egan
tw15egan approved these changes Jul 28, 2021
View reviewed changes
Copy link
Collaborator

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

LGTM 👍 ✅

@andreancardona
Copy link
Contributor Author

@tw15egan re-requested your review. Reverting the original PR: #9138
Sorry for the confusion!

@kodiakhq kodiakhq bot merged commit 32e41d9 into carbon-design-system:main Jul 28, 2021
andreancardona added a commit that referenced this pull request Jul 28, 2021
@andreancardona @joshblack @tw15egan
chore(release): v10.40.1 (#9352)
0b68ca7 
* hot-fix(pagination): fix style bug (#9346)

* fix(pagination): fix style bug

* reverting-inline-theming

Co-authored-by: Josh Black <[email protected]>
Co-authored-by: TJ Egan <[email protected]>

* chore(release): v10.40.0-rc.0

Co-authored-by: Josh Black <[email protected]>
Co-authored-by: TJ Egan <[email protected]>
@obambrough obambrough mentioned this pull request Aug 12, 2021
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tw15egan tw15egan tw15egan approved these changes

@joshblack joshblack joshblack approved these changes

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.
3 participants
@andreancardona @tw15egan @joshblack