Skip to content

Export useResizeObserver from @primer/react - #3027

Merged
joshblack merged 3 commits into
mainfrom
export-use-resize-observer
Mar 15, 2023
Merged

Export useResizeObserver from @primer/react#3027
joshblack merged 3 commits into
mainfrom
export-use-resize-observer

Conversation

@talum

@talumtalum commented Mar 14, 2023

Copy link
Copy Markdown
Contributor

Exporting the useResizeObserver hook from @primer/react so it can be used outside of Primer!

Closeshttps://github.com/github/primer/issues/1908

Screenshots

n/a

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@talumtalum self-assigned this Mar 14, 2023
@changeset-bot

changeset-botBot commented Mar 14, 2023•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 738c2a5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
@primer/reactMinor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actionsBot commented Mar 14, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js94.48 KB (+0.02% 🔺)
dist/browser.umd.js95.05 KB (+0.02% 🔺)

@talum
talum temporarily deployed to github-pages March 14, 2023 20:09 — with GitHub Actions Inactive
@github-actions
github-actionsBot temporarily deployed to storybook-preview-3027 March 14, 2023 20:09 Inactive
@talum
talum temporarily deployed to github-pages March 14, 2023 20:19 — with GitHub Actions Inactive
@github-actions
github-actionsBot temporarily deployed to storybook-preview-3027 March 14, 2023 20:19 Inactive
@joshblack

joshblack commented Mar 14, 2023

Copy link
Copy Markdown
Member

Hey @talum! 👋 Thanks so much for taking the time to make a PR for this 🙏

As a rule of thumb, when should we expect things be exported from Primer for hooks like this versus be packages in dotcom? (e.g.: https://gh.io/AAjq3a7) I wasn't sure and just wanted to double-check 👀

@talum

talum commented Mar 14, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@joshblack Great question, and I don't know about the answer! I'm following guidance from this comment: https://github.com/github/primer/issues/1908#issuecomment-1467375326

I would have thought it better to export this from Primer rather than implement a different version in dotcom. Interestingly, I don't even remember finding that in dotcom when I was searching. I definitely could use try using that hook in dotcom instead of exporting this one. What do y'all think?

🤔 Update: I think that one doesn't listen for resizing of a given element. Yeah, that one watches for window resizes whereas I care about resizing a certain element. So I think we do need this and I'd rather import it from a package rather than copying it over.

@github-actions
github-actionsBottemporarily deployed to storybook-preview-3027 March 14, 2023 21:07 Inactive
@talum
talum marked this pull request as ready for review March 14, 2023 21:11
@talum
talum requested review from a team and mperrottiMarch 14, 2023 21:11
colebemis
colebemis approved these changes Mar 14, 2023
View reviewed changes

@colebemiscolebemis 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.

Thank you! ✨

@joshblack

joshblack commented Mar 15, 2023

Copy link
Copy Markdown
Member

@talum totally! Makes sense 👍

@joshblack
joshblack added this pull request to the merge queueMar 15, 2023
Merged via the queue into main with commit 3153c81Mar 15, 2023
@joshblack
joshblack deleted the export-use-resize-observer branch March 15, 2023 15:56
@primer-cssprimer-css mentioned this pull request Mar 15, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshblackjoshblackjoshblack approved these changes
@mperrottimperrottiAwaiting requested review from mperrotti
+1 more reviewer
@colebemiscolebemiscolebemis approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@talumtalum

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@talum@joshblack@colebemis