PMM-14901: RTA details arrows respect filters#5397
Conversation
mattiasimonato
requested review from
fabio-silva and
matejkubinec
and removed request for
a team
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates RTA overview details-pane navigation so prev/next arrows traverse only the rows currently visible after table filtering (rather than the full API result set).
Changes:
OverviewTablenow exposes a filtered, pre-paginationnavigableQuerieslist derived from the MRT row model.RealtimeOverviewusesnavigableQueriesfor adjacent navigation and first/last button state.- Adds a unit test that mocks
OverviewTableto verify navigation ignores API-only (filtered-out) rows.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| ui/apps/pmm/src/pages/rta/overview/table/OverviewTable.tsx | Tracks table filter state and syncs a filtered/pre-pagination query list up to the parent for navigation. |
| ui/apps/pmm/src/pages/rta/overview/RealtimeOverview.tsx | Switches details-pane navigation to use the filtered query list rather than raw API results. |
| ui/apps/pmm/src/pages/rta/overview/RealtimeOverview.navigation.test.tsx | Adds tests ensuring next/prev navigation follows filtered rows, not the API’s full list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
matejkubinec
left a comment
matejkubinec
left a comment
There was a problem hiding this comment.
@mattiasimonato looks good outside of the one comment, one thing, do you think it would make sense to provide this functionality on the percona-ui level? (I've used a similar pattern on the alerts page POC, and it might be good to upstream it if the pattern will be used more often)
@pmcf-percona is the next/prev table item in a details pane a pattern that would be used on more parts of the application?
| } | ||
| ); | ||
| const [selectedQueryIndex, setSelectedQueryIndex] = useState<number>(); | ||
| const tableQueries = queries ?? EMPTY_QUERIES; |
There was a problem hiding this comment.
| const tableQueries = queries ?? EMPTY_QUERIES; | |
| const tableQueries: QueryData[] = queries ?? []; |
Or is there other reason to declare EMPTY_QUERIES?
There was a problem hiding this comment.
Likely, yes. So now it's in RTA and planned for Alerts. I see it potentially coming to Inventory as well, so this panel may be restricted to always consuming lists from MRT at Peak Design level
There was a problem hiding this comment.
@matejkubinec Yes, there’s a reason for EMPTY_QUERIES.
While queries is still undefined, queries ?? [] creates a new empty array on every render. That was making OverviewTable re-sync navigable rows over and over and caused a render loop (fixed in f84b62c). The module-level constant keeps the same empty array reference, so that doesn’t happen
There was a problem hiding this comment.
@mattiasimonato looks good outside of the one comment, one thing, do you think it would make sense to provide this functionality on the percona-ui level? (I've used a similar pattern on the alerts page POC, and it might be good to upstream it if the pattern will be used more often)
@pmcf-percona is the next/prev table item in a details pane a pattern that would be used on more parts of the application?
@matejkubinec Yeah, I think it makes sense to upstream this if we’ll reuse it. I’d rather not block this PR on a percona-ui change though.
If you’re good with that, let’s add a follow-up ticket 👍
There was a problem hiding this comment.
@matejkubinec Yes, there’s a reason for
EMPTY_QUERIES.While queries is still undefined,
queries ?? []creates a new empty array on every render. That was makingOverviewTablere-sync navigable rows over and over and caused a render loop (fixed in f84b62c). The module-level constant keeps the same empty array reference, so that doesn’t happen
Yeah, I thought it would be something like that.
There was a problem hiding this comment.
@mattiasimonato looks good outside of the one comment, one thing, do you think it would make sense to provide this functionality on the percona-ui level? (I've used a similar pattern on the alerts page POC, and it might be good to upstream it if the pattern will be used more often)
@pmcf-percona is the next/prev table item in a details pane a pattern that would be used on more parts of the application?@matejkubinec Yeah, I think it makes sense to upstream this if we’ll reuse it. I’d rather not block this PR on a percona-ui change though.
If you’re good with that, let’s add a follow-up ticket 👍
Yes, let's create a follow up.
PMM-14901
Link to the Feature Build: SUBMODULES-4370
Fixes RTA overview details pane navigation so prev/next arrows move only through filtered table rows, not the full query list from the API.
OverviewTablesyncsnavigableQueriesfrom Material React Table (after column/global filters, before pagination).RealtimeOverviewuses that list for arrow navigation and first/last button state.