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

sidenav: Fix props leaking to DOM on Pagination component #2057

Merged
merged 3 commits into from Jan 14, 2022

Conversation

lachlanjc
Copy link
Member

@lachlanjc lachlanjc commented Dec 30, 2021

Closes #1471.

On production docs:
987A3ED6-E320-4028-AC0F-FE47F3A328B6

On this branch:
519700BA-0727-42E8-943C-BBA6F9601DB9

Looking at our usage of this component, I can’t see any breaking changes from not spreading these props, but please flag if you are aware of any!

📦 Published PR as canary version: 0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0

✨ Test out this PR locally via:

npm install @theme-ui/color-modes@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/color@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/components@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/core@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/css@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/custom-properties@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/editor@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install gatsby-plugin-theme-ui@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install gatsby-theme-style-guide@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install gatsby-theme-ui-layout@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/match-media@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/mdx@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/parse-props@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-base@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-bootstrap@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-bulma@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-dark@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-deep@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-funk@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-future@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-polaris@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-roboto@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-sketchy@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-swiss@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-system@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-tailwind@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/preset-tosh@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/presets@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/prism@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/sidenav@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/style-guide@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/tailwind@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/theme-provider@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install theme-ui@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
npm install @theme-ui/typography@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
# or 
yarn add @theme-ui/color-modes@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/color@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/components@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/core@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/css@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/custom-properties@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/editor@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add gatsby-plugin-theme-ui@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add gatsby-theme-style-guide@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add gatsby-theme-ui-layout@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/match-media@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/mdx@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/parse-props@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-base@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-bootstrap@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-bulma@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-dark@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-deep@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-funk@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-future@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-polaris@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-roboto@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-sketchy@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-swiss@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-system@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-tailwind@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/preset-tosh@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/presets@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/prism@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/sidenav@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/style-guide@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/tailwind@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/theme-provider@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add theme-ui@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0
yarn add @theme-ui/typography@0.13.2--canary.2057.053c111b329c4bf405fcc03dc304d334b3ece53e.0

@lachlanjc lachlanjc added the affects-docs Changes affect documentation, but not only documentation label Dec 30, 2021
@vercel
Copy link

vercel bot commented Dec 30, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/systemui/theme-ui/9x2oWr7vXwedAdZVXwuFGBxHMbxV
✅ Preview: https://theme-ui-git-lachlanjc-pagination-leak-systemui.vercel.app

@hasparus
Copy link
Member

Technically, this package is user-facing. Some of those 290 weekly downloads could pass style className or css prop to that div. Or even an id.

I'm not very opinionated about this, but shouldn't we instead fix it in the docs package? The may be something I'm missing, I'm not very familiar with this part of the codebase.

Tip: If you add a section starting from ### Release notes to your post, the bot will add it to changelog in GitHub Releases.

@lachlanjc
Copy link
Member Author

That's true. Will explore. Thanks for the tip!

@lachlanjc
Copy link
Member Author

@hasparus made this non-breaking!

@lachlanjc lachlanjc mentioned this pull request Jan 13, 2022
@hasparus hasparus merged commit a0c2d6b into develop Jan 14, 2022
@hasparus hasparus deleted the lachlanjc-pagination-leak branch January 14, 2022 12:27
@hasparus hasparus added the prerelease This change is available in a prerelease. label Jan 15, 2022
@hasparus
Copy link
Member

🚀 PR was released in v0.14.0 🚀

@hasparus hasparus added released This issue/pull request has been released. and removed prerelease This change is available in a prerelease. labels Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-docs Changes affect documentation, but not only documentation released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

properties leaking through
2 participants