#765 Results Page: Persist the column filters in the results table in the url #778

Netacci · 2024-10-09T19:51:46Z

Description

This PR addresses the issue where the results table resets during a page reload after a user applies column filters (e.g., platform, status, confidence). The filtered values do not persist, causing the results table to revert to its default state.

Before

perf.mp4

The problem occurred because the filters were not stored in the URL, leading to their loss upon page reload or when the URL is shared. As a result, the filtered state is not retained, and users have to reapply their filters each time a page reloads.

Test cases

By default, all filter values are checked, and as a result, all filter values are appended to the URL parameters.

To achieve this, I used useEffect to set an initial filter state. This useEffect sets all the checked filter values to the URL.

defau-1.mp4

When specific filters are applied or modified, the relevant filter values are added or updated in the URL.

I adjusted the onToggle function to ensure that filter values are added to the URL when they are checked and removed when
unchecked. This helps in maintaining persistence on page reload.

add-remove-filters-1.mp4

On page reload or URL sharing, filtered values appended to the URL remain the same thereby persisting the results table filtered state.

page-reload-1.mp4

If all filters are unchecked, the results table retains the unfiltered state, and the URL maintains the filter parameter keys with empty values.

This was challenging to implement at first, because when all filter values are unchecked and the page reloads, the default behavior (from test case 1) would normally re-check all filters and set them back to checked. To resolve this, instead of removing the parameter keys entirely from the URL, I set their values to empty. I also updated the useEffect to run once on page load and check if there is existing checked values to be applied before setting the default. That way there's a significant difference between when a user has unchecked all filters and when the default filter settings should apply, ensuring the correct behavior on page load.

all-unchecked-1.mp4

When the clear filter button is clicked, filtered values are added back to the URL.
I updated the clearFilter function to set all filter values to the URL.

clear-filter-1.mp4

Steps to test

Navigate to results table page
Apply Column filters by platform, status and/or confidence
Refresh the page and observe the filters persist both in the URL and the results table
Copy the URL to another tab and observe the filtered state is still maintained.

netlify · 2024-10-09T19:52:06Z

✅ Deploy Preview for mozilla-perfcompare ready!

Name	Link
🔨 Latest commit	`01d28de`
🔍 Latest deploy log	https://app.netlify.com/sites/mozilla-perfcompare/deploys/671a50eb5ebfdb0008bec433
😎 Deploy Preview	https://deploy-preview-778--mozilla-perfcompare.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

syedbarimanjan

Left some comments.

src/__tests__/CompareResults/__snapshots__/OverTimeResultsView.test.tsx.snap

src/__tests__/CompareResults/__snapshots__/ResultsTable.test.tsx.snap

src/__tests__/CompareResults/__snapshots__/ResultsView.test.tsx.snap

src/components/CompareResults/ResultsTable.tsx

codecov · 2024-10-10T12:40:12Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.55%. Comparing base (01b30d1) to head (64fd51e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #778      +/-   ##
==========================================
+ Coverage   91.53%   91.55%   +0.02%     
==========================================
  Files          88       88              
  Lines        2350     2369      +19     
  Branches      443      449       +6     
==========================================
+ Hits         2151     2169      +18     
- Misses        176      177       +1     
  Partials       23       23

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

julienw

Thanks, this looks pretty good! The implementation is a bit complex though, so I left some comments about it, tell me what you think!

src/types/types.ts

src/components/CompareResults/ResultsTable.tsx

src/__tests__/CompareResults/ResultsView.test.tsx

julienw · 2024-10-18T16:38:47Z

src/components/CompareResults/ResultsTable.tsx

+  useEffect(() => {
+    accessURlParams();
+  }, []);


Instead of using an effect, it would be better to use the 3rd version of useState (as described in the react documentation: https://react.dev/reference/react/useState#usestate).

This would set the initial value of tableFilters by using the URL Params directly.

By using an effect, instead, this impairs the performance. Indeed this works like this:

React renders the component (and the rest of the tree)

React runs the effect.

If the effect changes the state, React renders the component again... and the rest of the tree as well!

We want to avoid 2 and 3. By setting the right state right away at the first render time, only the first step is run, so this is much faster.

I'd like to stress that you need to use the 3rd version of useState, not the first or second. Why? Because we want the computation to happen just once. This is explained in this part of their documentation: https://react.dev/reference/react/useState#avoiding-recreating-the-initial-state

@julienw Thank you for the detailed explanation!

I’ve updated the code to use lazy initialization for tableFilters based on the URL Params as suggested. This way, the computation only happens once, and React renders the component only once during the initial render.

Thanks again for pointing this out, and for the useful documentation links!

Hi @julienw I have implemented these changes and waiting for a re-review

- If URL is empty, then all table filters are checked, otherwise set the table filters based off that. This helps results table to persist upon reload and url copying - Created a function that sets the filter to the url params on toggle

julienw

Thanks, this looks pretty good!

I just noticed a few small things that could be improved, and also a test for the functionality is missing.

That's OK to not fix this PR, please focus on your new task, but I still wanted to give you this last feedback.

julienw · 2024-10-24T18:35:59Z

src/components/CompareResults/ResultsTable.tsx

+  };
+
+  useEffect(() => {
+    if (tableFilters.size > 0) {


This if condition shouldn't be necessary: indeed you also want to update the URL when the value changes and is empty :-)

Yes, I see what you mean. I'll update this

julienw · 2024-10-24T18:36:31Z

src/components/CompareResults/ResultsTable.tsx

+      rawSearchParams.delete(`filter-${columnId}`);
+      updateRawSearchParams(rawSearchParams);


This should be unnecessary with the effect above especially if you remove the if.

julienw · 2024-10-24T18:38:25Z

src/components/CompareResults/ResultsTable.tsx

+    cellsConfiguration.forEach(({ key, possibleValues }) => {
+      if (!possibleValues) return;
+      const paramValue = rawSearchParams.get(`filter-${key}`);
+      initialFilters.set(key, new Set(paramValue?.split(',')));


it would be good to not set anything if paramValue is not present

Carla-Moz · 2024-10-24T18:47:10Z

@Netacci Hi I'd like to let you know you've been selected to go on the next round! Congrats!
Please see the matrix channel for details https://chat.mozilla.org/#/room/#perfcompare-outreachy:mozilla.org

Netacci · 2024-10-24T18:49:18Z

@Netacci Hi I'd like to let you know you've been selected to go on the next round! Congrats!
Please see the matrix channel for details https://chat.mozilla.org/#/room/#perfcompare-outreachy:mozilla.org

Thanks @Carla-Moz this is great news 🥲 I appreciate this opportunity 🙏

Netacci · 2024-10-24T18:59:49Z

Thanks, this looks pretty good!

I just noticed a few small things that could be improved, and also a test for the functionality is missing.

That's OK to not fix this PR, please focus on your new task, but I still wanted to give you this last feedback.

Okay thanks for this.

Carla-Moz added the Outreachy-PR label Oct 10, 2024

syedbarimanjan reviewed Oct 10, 2024

View reviewed changes

Netacci marked this pull request as draft October 10, 2024 11:00

Netacci force-pushed the persist-result-table-filters branch from b618a8d to 2ae22b0 Compare October 10, 2024 12:39

Netacci marked this pull request as ready for review October 10, 2024 13:02

Netacci force-pushed the persist-result-table-filters branch 2 times, most recently from cdb7795 to 3b37a51 Compare October 10, 2024 18:25

Netacci closed this Oct 11, 2024

Netacci reopened this Oct 11, 2024

julienw requested changes Oct 11, 2024

View reviewed changes

julienw reviewed Oct 18, 2024

View reviewed changes

Netacci requested a review from julienw October 22, 2024 07:51

Netacci force-pushed the persist-result-table-filters branch from f12d5fe to 64fd51e Compare October 23, 2024 07:26

Netacci force-pushed the persist-result-table-filters branch from 64fd51e to 01d28de Compare October 24, 2024 13:51

julienw reviewed Oct 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#765 Results Page: Persist the column filters in the results table in the url #778

#765 Results Page: Persist the column filters in the results table in the url #778

Netacci commented Oct 9, 2024

netlify bot commented Oct 9, 2024 •

edited

Loading

syedbarimanjan left a comment

codecov bot commented Oct 10, 2024 •

edited

Loading

julienw left a comment

julienw Oct 18, 2024

Netacci Oct 18, 2024

Netacci Oct 24, 2024

julienw left a comment

julienw Oct 24, 2024

Netacci Oct 24, 2024

julienw Oct 24, 2024

julienw Oct 24, 2024

Carla-Moz commented Oct 24, 2024

Netacci commented Oct 24, 2024

Netacci commented Oct 24, 2024

		rawSearchParams.delete(`filter-${columnId}`);
		updateRawSearchParams(rawSearchParams);

#765 Results Page: Persist the column filters in the results table in the url #778

Are you sure you want to change the base?

#765 Results Page: Persist the column filters in the results table in the url #778

Conversation

Netacci commented Oct 9, 2024

Description

Test cases

Steps to test

netlify bot commented Oct 9, 2024 • edited Loading

✅ Deploy Preview for mozilla-perfcompare ready!

syedbarimanjan left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 10, 2024 • edited Loading

Codecov Report

julienw left a comment

Choose a reason for hiding this comment

julienw Oct 18, 2024

Choose a reason for hiding this comment

Netacci Oct 18, 2024

Choose a reason for hiding this comment

Netacci Oct 24, 2024

Choose a reason for hiding this comment

julienw left a comment

Choose a reason for hiding this comment

julienw Oct 24, 2024

Choose a reason for hiding this comment

Netacci Oct 24, 2024

Choose a reason for hiding this comment

julienw Oct 24, 2024

Choose a reason for hiding this comment

julienw Oct 24, 2024

Choose a reason for hiding this comment

Carla-Moz commented Oct 24, 2024

Netacci commented Oct 24, 2024

Netacci commented Oct 24, 2024

netlify bot commented Oct 9, 2024 •

edited

Loading

codecov bot commented Oct 10, 2024 •

edited

Loading