ui: save filters on cache for Transactions page

[maryliag]

Previously, a sort selection was not maintained when
the page change (e.g. changing tabs/pages).
This commits saves the selected value to be used.

Partially adresses #71851

Release note: None

[cockroach-teamcity]

This change is 

[Azhng]

Nice refactoring! 🚀. Just some nits and questions

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Azhng and @maryliag)

---

pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx, line 116 at r1 (raw file):

/**
 * Get Filters from Query String and if it's different from current
 * filters calls the onFilterChange function.

"if it's different from filters calls the onFilterChange function", something feels missing in that sentence

---

pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx, line 131 at r1 (raw file):

  const filtersQueryString = getFiltersFromQueryString(history.location.search);
  const searchParams = new URLSearchParams(history.location.search);
  const hasFilter = searchParams.get("app") || undefined;

quick question: why are we only check "app"here? :thinking: , doesn't thesearchParams` also include other things like "node" and "regions" etc. ?

---

pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx, line 466 at r1 (raw file):

export const updateSortSettingQueryParamsOnTab = (
  tab: string,
  ss: SortSetting,

nit: s/ss/sortSetting/? I feel we don't really have much abbreviation across the entire code base

---

pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx, line 467 at r1 (raw file):

  tab: string,
  ss: SortSetting,
  defaultSS: SortSetting,

ditto

[maryliag]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Azhng)

---

pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx, line 116 at r1 (raw file):

Previously, Azhng (Archer Zhang) wrote…

"if it's different from filters calls the onFilterChange function", something feels missing in that sentence

Changed

---

pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx, line 131 at r1 (raw file):

Previously, Azhng (Archer Zhang) wrote…

quick question: why are we only check "app"here? :thinking: , doesn't thesearchParams` also include other things like "node" and "regions" etc. ?

Good catch, initially I was always adding app, so it was easy to use that to check if we had any filter, but I updated now so it checks for any filter

---

pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx, line 466 at r1 (raw file):

Previously, Azhng (Archer Zhang) wrote…

nit: s/ss/sortSetting/? I feel we don't really have much abbreviation across the entire code base

done

---

pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx, line 467 at r1 (raw file):

Previously, Azhng (Archer Zhang) wrote…

ditto

done

[Azhng]

Reviewed 1 of 11 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @Azhng and @maryliag)

---

pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx, line 131 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

Good catch, initially I was always adding app, so it was easy to use that to check if we had any filter, but I updated now so it checks for any filter

Nice!

---

pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx, line 179 at r2 (raw file):

 * @param history
 */
export const updateFiltersQueryParamsOnTab = (

same nit

---

pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx, line 421 at r2 (raw file):

/**
 * Get Sort Setting from Query String and if it's different from current
 * sortSetting calls the onSortChange function.

super nit: "if it's different ...., then calls the onSortChange function"

---

pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx, line 464 at r2 (raw file):

 * @param history
 */
export const updateSortSettingQueryParamsOnTab = (

super nit: s/updateSortSettingQueryParamsOnTab/updateSortSettingQueryParamsOnTabChange/

---

pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx, line 182 at r2 (raw file):

  }

  updateQueryParamsOnTabSwitch(): void {

Hmm, it seems like we are updating query params more than just when we switch tabs? So perhaps we can drop OnTabSwitch suffix here?

[maryliag]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @Azhng)

---

pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx, line 179 at r2 (raw file):

Previously, Azhng (Archer Zhang) wrote…

same nit

same as other comment

---

pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx, line 421 at r2 (raw file):

Previously, Azhng (Archer Zhang) wrote…

super nit: "if it's different ...., then calls the onSortChange function"

Done.

---

pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx, line 464 at r2 (raw file):

Previously, Azhng (Archer Zhang) wrote…

super nit: s/updateSortSettingQueryParamsOnTab/updateSortSettingQueryParamsOnTabChange/

I named this on purpose, because it's not on the event of TabChange, is during any updated events when you're on a specific Tab. So I'm keeping this name as is :)

---

pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx, line 182 at r2 (raw file):

Previously, Azhng (Archer Zhang) wrote…

Hmm, it seems like we are updating query params more than just when we switch tabs? So perhaps we can drop OnTabSwitch suffix here?

Done.

[maryliag]

bors r+

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build succeeded:

• GitHub CI (Cockroach)