-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: Refactor <TablePageLayout> to support multiple tabs #911
Conversation
Do we think people may have the |
return prev | ||
}, | ||
{ replace: true }, | ||
) | ||
} | ||
}, [filteredCount, page, setSearchParams]) | ||
}, [filteredCount, page, pageQueryParam, setSearchParams]) |
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.
It's confusing that page
and pageQueryParam
are two different things. Can we rename one?
tableTypeQueryParam
? dataTypeQueryParam
? 🤔
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.
Renamed both.
Definitely a good thing to keep in mind. Right now nothing on the runs page ever exceeds 1 page so we should be good. |
title={i18n.filterNoResultsFound} | ||
description={i18n.filterTooRestrictive} | ||
actions={<Button onClick={reset}>{i18n.clearFilters}</Button>} | ||
/> | ||
), | ||
filteredCount: filteredDatasetCount, | ||
totalCount: datasetCount, | ||
countLabel: i18n.datasets, |
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.
while you're here, would you mind updating to using useI18n()
/t
?
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.
Done.
🤖 I have created a release *beep* *boop* --- ## [1.14.0](web-v1.13.1...web-v1.14.0) (2024-07-25) ### ✨ Features * Refactor <TablePageLayout> to support multiple tabs ([#911](#911)) ([659bea3](659bea3)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
#624
This is to support having both Annotations and Tomograms tabs on the Runs page.
<Tabs>
component.page
toannotations-page
. In a subsequent PR I'll addtomograms-page
.<Tabs>
to take in components as labels andnumber
s (i.e. indexes) as values.