Skip to content

feat(TreeView): add support for empty parent nodes - #2478

Merged
joshblack merged 7 commits into
mainfrom
feat/add-empty-load-case
Oct 24, 2022
Merged

feat(TreeView): add support for empty parent nodes#2478
joshblack merged 7 commits into
mainfrom
feat/add-empty-load-case

Conversation

@joshblack

@joshblackjoshblack commented Oct 24, 2022

Copy link
Copy Markdown
Member

This PR updates TreeView.Item and TreeView.SubTree based on the feedback over in: https://github.com/github/primer/pull/1389#discussion_r998754809

Changelog

New

  • Add EmptyDirectory story for TreeView

Changed

  • Update TreeView.Item and TreeView.SubTree to coordinate whether or not aria-expanded should be set in sync and async contexts
  • Update TreeView.Item to announce when a TreeView.SubTree has no content when used in an async context
  • Update TreeView.Item to shift conditional rendering of TreeView.SubTree into TreeView.SubTree
  • Update TreeView.test.tsx to remove it.only and replace timers with jest.useFakeTimers

Removed

@changeset-bot

changeset-botBot commented Oct 24, 2022

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 19a9f0a

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

@joshblack
joshblack marked this pull request as ready for review October 24, 2022 17:35
@joshblack
joshblack requested review from a team, colebemis and mperrottiOctober 24, 2022 17:35
@github-actions

github-actionsBot commented Oct 24, 2022

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js77.99 KB (0%)
dist/browser.umd.js78.64 KB (0%)

@joshblack
joshblack temporarily deployed to github-pages October 24, 2022 17:40 Inactive
@joshblack
joshblack temporarily deployed to github-pages October 24, 2022 19:53 Inactive
Comment threadsrc/TreeView/TreeView.test.tsx Outdated

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

Looks great! Could you add a couple of tests to make sure the aria-expanded attribute is behaving as expected in the sync and async cases?

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

Thanks for tackling this tricky implementation 💖

Should be ready to merge after fixing the lint error.

@joshblack
joshblack enabled auto-merge (squash) October 24, 2022 21:03
@joshblack
joshblack temporarily deployed to github-pages October 24, 2022 21:06 Inactive
@joshblack
joshblack merged commit 7cbb5c9 into mainOct 24, 2022
@joshblack
joshblack deleted the feat/add-empty-load-case branch October 24, 2022 21:10
@primer-cssprimer-css mentioned this pull request Oct 24, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@joshblack@colebemis