Skip to content

Use SSR-compatible slot implementation in TreeView - #3097

Merged
colebemis merged 5 commits into
mainfrom
treeview-slots
Mar 31, 2023
Merged

Use SSR-compatible slot implementation in TreeView#3097
colebemis merged 5 commits into
mainfrom
treeview-slots

Conversation

@colebemis

@colebemiscolebemis commented Mar 30, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Swaps out the underlying slots implementation for TreeView.LeadingVisual and TreeView.TrailingVisual.

See #1690 for more context.

@changeset-bot

changeset-botBot commented Mar 30, 2023•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3cc1985

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

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

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 30, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js95.04 KB (-0.06% 🔽)
dist/browser.umd.js95.58 KB (-0.06% 🔽)

@colebemiscolebemis mentioned this pull request Mar 30, 2023
Closed
10 tasks
@github-actions
github-actionsBot temporarily deployed to storybook-preview-3097 March 30, 2023 16:40 Inactive
@primer
primerBot temporarily deployed to github-pages March 30, 2023 16:40 Inactive
@github-actions
github-actionsBot temporarily deployed to storybook-preview-3097 March 30, 2023 16:41 Inactive
@colebemis
colebemis temporarily deployed to github-pages March 30, 2023 17:53 — with GitHub Actions Inactive
@github-actions
github-actionsBot temporarily deployed to storybook-preview-3097 March 30, 2023 17:53 Inactive
@colebemis
colebemis marked this pull request as ready for review March 30, 2023 18:04
@colebemis
colebemis requested review from a team and josepmartinsMarch 30, 2023 18:04
@colebemis
colebemis temporarily deployed to github-pages March 30, 2023 18:18 — with GitHub Actions Inactive
@github-actions
github-actionsBot temporarily deployed to storybook-preview-3097 March 30, 2023 18:19 Inactive
colebemis
colebemis commented Mar 31, 2023
View reviewed changes
Comment thread.changeset/sharp-eggs-crash.md Outdated
@github-actions
github-actionsBottemporarily deployed to storybook-preview-3097 March 31, 2023 14:47 Inactive
mperrotti
mperrotti approved these changes Mar 31, 2023
View reviewed changes

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

Are there any new tests we should write to test the SSR compatibility? If not, LGTM!

These changes look great, and everything looks good in Storybook. However, I didn't test the SSR compatibility because I'm not sure how to in this repo.

@colebemis

colebemis commented Mar 31, 2023

Copy link
Copy Markdown
ContributorAuthor

Are there any new tests we should write to test the SSR compatibility? If not, LGTM!

These changes look great, and everything looks good in Storybook. However, I didn't test the SSR compatibility because I'm not sure how to in this repo.

We have tests to ensure useSlots work as expected. I don't think it's necessary to explicitly test that functionality in TreeView.

@colebemis
colebemis added this pull request to the merge queueMar 31, 2023
Merged via the queue into main with commit a38c00eMar 31, 2023
@colebemis
colebemis deleted the treeview-slots branch March 31, 2023 19:35
@primer-cssprimer-css mentioned this pull request Mar 31, 2023
Merged
@primer-cssprimer-css mentioned this pull request Apr 10, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@josepmartinsjosepmartinsAwaiting requested review from josepmartins
@siddharthkpsiddharthkpAwaiting requested review from siddharthkp
1 more reviewer
@mperrottimperrottimperrotti approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@colebemis@mperrotti