AdvancedTable: fix styles for right/center aligned cells
#3093
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
dchyun
left a comment
dchyun
left a comment
There was a problem hiding this comment.
nice catch lgtm! Note: Since this does have a slight dom change will need to check in the next release testing that no consumer tests break, but should be unlikely.
shleewhite
force-pushed
the
hds-5254/advanced-table-align
branch
from
f4edbca to
a224735
Compare
shleewhite
force-pushed
the
hds-5254/advanced-table-align
branch
from
8b4b414 to
f7cbd74
Compare
| <Hds::AdvancedTable::Th | ||
| @isExpandable={{true}} | ||
| @tooltip="Here is more information" | ||
| @column={{get this.sampleTableModel.columns 1}} |
There was a problem hiding this comment.
Note: for both ThSort and Th, there is a scrollbar for this example. This is because of the visually hidden resize handler. I think it is ok for now.
There was a problem hiding this comment.
Just wondering... Does it make sense to have a resize handle on the last cell in a row?
There was a problem hiding this comment.
When I look at the "Resizable columns with sorting" example, the last column is not resizable (does not have a resize handle on the right-most side of the table) which is what I would expect. Could you possibly update your example to get rid of the resize handle on the last row?
There was a problem hiding this comment.
Ahh ok I see. I need to include the resize handle for the context menu to show up, so I will just add another column that doesnt have one.
dchyun
left a comment
dchyun
left a comment
There was a problem hiding this comment.
New showcase examples are really helpful. I'm seeing one issue present previously with the tooltip alignment, but that's it.
dchyun
left a comment
dchyun
left a comment
There was a problem hiding this comment.
Looking good! All percy diff looks expected.
| </SF.Item> | ||
| </Shw::Flex> | ||
|
|
||
| <Shw::Flex @label="Alignment with all interactive elements" as |SF|> |
There was a problem hiding this comment.
Nice! Separating the examples out like this makes it much clearer.
KristinLBradley
left a comment
KristinLBradley
left a comment
There was a problem hiding this comment.
I think this is looking good and the Percy diffs look as expected. I left comments related to the overflow scrolling & resize handle. I think you could just remove the resize on the last column to fix the issue.
There was a problem hiding this comment.
[todo][non-blocker] I can make this update after this merges, but one small unrealted fix is in the Td examples for the inline dropdown the route needs to be updated to page-components.advanced-table. Didn't realize I'd been routed to the table page after clicking it and was quite confused for a min 😅.
<D.Interactive @route="page-components.table">Dropdown</D.Interactive>
There was a problem hiding this comment.
hahhaha way ahead of you, I already fixed it in the spike branch.
6 participants
📌 Summary
If merged, this PR would fix the styles when the user sets
@alignto"right"or"center".PREVIEW
📸 Screenshots
CURRENT PRODUCTION SHOWCASE
AFTER PROPOSED FIX
🔗 External links
Jira ticket: HDS-5254
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.
📋 PCI review checklist
Examples of changes to controls include access controls, encryption, logging, etc.
Examples include changes to operating systems, ports, protocols, services, cryptography-related components, PII processing code, etc.