Column header with menu alignment#3907
Conversation
snowystinger
changed the title
|
Should the alignment of the menu itself also change? Looks like it's still start aligned. |
|
@devongovett the one odd case is center alignment since we don't have center alignment of MenuTrigger, though maybe this is a good reason to expose that? thoughts? |
|
Ehhhh I think it's fine as is, doesn't feel like a big enough deal to add a whole new feature to menu |
|
Build successful! 🎉 |
|
## API Changes
unknown top level export { type: 'identifier', name: 'Column' } |
LFDanLu
left a comment
LFDanLu
left a comment
There was a problem hiding this comment.
Alignment looks good to me, tested in various locales + column sizes. Some things I noticed when testing, just thought I'd bring them up
| let showResizer = !isEmpty && ((headerRowHovered && getInteractionModality() !== 'keyboard') || resizingColumn != null); | ||
| let alignment = 'start'; | ||
| let menuAlign = 'start' as 'start' | 'end'; | ||
| if (columnProps.align === 'center' || column.colspan > 1) { |
There was a problem hiding this comment.
Unrelated to the PR, but this reminded me that nested columns were a thing so I tried adding resizing to those stories:
Noted that I couldn't resize the parent columns, not sure we want to support that right now/may need to discuss the behavior there. Do we wanna disable resizing for columns with children for now? Happy to open a PR if so.
There was a problem hiding this comment.
Yeah, I have not done anything with parent columns, I'm not sure how that would work. Maybe it'd be good to disable it from being an option to even set that prop somehow...
There was a problem hiding this comment.
How about we add a warning in TableView if column.hasChildNodes + allowsResizing is provided. We could then add a couple checks in other places to prevent a column from rendering as a resizable column if it has childNodes
There was a problem hiding this comment.
I guess, do actions still matter for combined columns?
I'll merge this one and make a follow up to discuss
4 participants
Closes #3880
✅ Pull Request Checklist:
📝 Test Instructions:
Go to new story, ?path=/story/tableview--should-fill-cell-width-allowsresizing
column headers should all match as they do in the non-resizing story just before it, it may be easiest to have both stories open and switch between the tabs
Menu chevron should always be visible if there is a menu
🧢 Your Project: