TreeView: Align tree item toggle and visual icons to top of item
#4572
Conversation
🦋 Changeset detectedLatest commit: 39c1f6e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
iansan5653
changed the title
TreeView: Align tree item toggle icon to top of item
size-limit report 📦
|
There was a problem hiding this comment.
Hey @iansan5653 👋🏻 Thanks for the PR! Left a comment about horizontal centring. The rest of it looks good to me. We should also get a designer review on this though - tagging @mperrotti 🙌🏻
|
@iansan5653 - can we add a story that demonstrates this change? Kind of like this... Alternatively, we could update all existing stories to have an expandable node that has 2 lines of content. That seems unnecessary though... |
|
Uh oh! @mperrotti, the image you shared is missing helpful alt text. Check #4572 (comment). Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
|
|
Added a story. Now that I'm looking at it I'm thinking maybe we should top-align the leading and trailing visuals as well? I'm not totally sure how I feel about this approach given the leading/trailing visual appearance... maybe overriding the styling for the specific use case of sub-issues is actually better 🤔 But we don't have any official style overriding support, requiring us to reach in and select 
|
|
Uh oh! @iansan5653, the image you shared is missing helpful alt text. Check #4572 (comment). Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
|
It makes sense o me to align all to the top - not the same case but we took a similar approach for pageheader as well that we align all visuals to the top when the title is multiline @mperrotti thoughts? |
If that's the preferred approach I'd be happy to implement it 👍 |
|
I updated the leading/trailing visual to be top aligned as well: 185b613. I removed the Edit: VRT in CI was failing so I added the
|
|
Uh oh! @JelloBagel, the image you shared is missing helpful alt text. Check #4572 (comment). Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
|
JelloBagel
changed the title
TreeView: Align tree item toggle icon to top of itemTreeView: Align tree item toggle and visual icons to top of item
The vast majority of the time,
TreeViewitems are only one line and the content is truncated to fit. However, they can potentially wrap, as in https://github.com/github/collaboration-workflows-flex/issues/902 (internal issue). In this case, we want the chevron to still remain locked to the top of the item, rather than vertically centered.This is slightly challenging because the height of items changes depending on the input mode, expanding automatically for touch controls. It wasn't complicated to move the
min-heightto a CSS variable and calculate a correct padding based on that:min-height / 2 - icon-height.There is no visual change here for the non-wrapped item.
Changelog
New
Changed
TreeViewchevron icon to top of itemRemoved
Rollout strategy
Testing & Reviewing
Merge checklist