-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(ui): Call table footer - pagination sizing, call-out for Alt/Option command #3305
base: master
Are you sure you want to change the base?
Conversation
Preview this PR with FeatureBee: https://beta.wandb.ai/?betaVersion=a75b0313281e43afce4f78f80363e1b6fdb0b6ca |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small changes requested:
for small table sizes we probably want this text to be hidden (or something, maybe tooltip?) instead of wrapping.
When the table is being shown in a preview, like in the details drawer, we don't need to show this callout. It just adds complexity and does nothing (cells aren't clickable here).
@@ -392,7 +392,7 @@ export const DataTableView: FC<{ | |||
const displayRows = 10; | |||
const hideFooter = USE_TABLE_FOR_ARRAYS && gridRows.length <= displayRows; | |||
const headerHeight = 40; | |||
const footerHeight = 52; | |||
const footerHeight = 40; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the height of the footer bar in px after the smaller button change. It gets passed to help calculate the height of the datatable above it.
display="flex" | ||
alignItems="center" | ||
marginLeft="auto" | ||
marginRight={1} | ||
sx={{fontSize: '14px', color: MOON_500}}> | ||
<Box | ||
component="span" | ||
sx={{ | ||
fontSize: '12px', | ||
border: `1px solid ${MOON_400}`, | ||
fontWeight: '600', | ||
padding: '0px 4px', | ||
marginRight: '4px', | ||
borderRadius: '4px', | ||
}}> | ||
{isMac ? 'Option' : 'Alt'} | ||
</Box> | ||
+ | ||
<Box | ||
component="span" | ||
sx={{ | ||
fontSize: '12px', | ||
border: `1px solid ${MOON_400}`, | ||
fontWeight: '600', | ||
padding: '0px 4px', | ||
marginLeft: '4px', | ||
marginRight: '4px', | ||
borderRadius: '4px', | ||
}}> | ||
Click | ||
</Box> | ||
on a cell to filter by the value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this could be its own decomposed component. name suggestions: "CellSortCallout", "PaginationControlsSortCallout" etc
Yes, will update - should hide it away when the screen is too wide and on the child views. Will also make it its own component 👍 |
Description
isMac
command to use utils (same as here).Screenshot
Before:
After: