Skip to content
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

Persist Column Filters in URL (#765) #788

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ST-KO
Copy link

@ST-KO ST-KO commented Oct 17, 2024

This PR refactors the onToggleFilter and onClearFilter functions by moving the URL persistence logic out of the setTableFilters callback, ensuring better separation of concerns and cleaner code structure. The functionality remains the same as my previous PR (#783), but with improved implementation.

This new PR replaces the previous one (#783) due to issues with CircleCI tests.

Overview:

This pull request implements functionality to persist column filters in the URL. Users can now refresh the page and maintain their selected filters, improving the overall user experience. This fixes issue #765.

Changes Made:

  1. Filters Persist in the URL

Implemented the useEffect hook that reads the filter parameters from the URL when the component mounts, ensuring that the selected filters are applied even after a page refresh.

  • Before
Filters.do.not.persist.when.page.refresh.mov
  • After
Filters.persist.when.page.refresh.mp4

Shared Filtered View URL

Update the URL with the applied filters in the existing onToggleFilter method, allowing users to share the URL and they will see the same filtered results.

  • Before
Shared.URL.does.not.provide.same.results.mov
  • After
Shared.URL.provides.the.same.results.for.all.users.mov

URL Updates as Filters are Applied or Removed

This functionality is implemented in the existing onToggleFilter and onClearFilter methods, where parameters are dynamically added or deleted. I utilised the existing useRawSearchParams custom hook to prevent unnecessary page re-rendering. This ensures that the URL dynamically updates as filters are added or removed.

  • Before
URL.does.not.update.as.filters.are.applied.or.removed.mov
  • After
URL.updates.as.filters.are.applied.or.removed.mp4

Copy link

netlify bot commented Oct 17, 2024

Deploy Preview for mozilla-perfcompare ready!

Name Link
🔨 Latest commit 69fb47d
🔍 Latest deploy log https://app.netlify.com/sites/mozilla-perfcompare/deploys/671add9f54984c0008ff447e
😎 Deploy Preview https://deploy-preview-788--mozilla-perfcompare.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@@ -107,7 +107,26 @@ export default function ResultsTable() {
new Map() as Map<string, Set<string>>, // ColumnID -> Set<Values to remove>
);

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a brief comment description before this useEffect?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your review. I have added comment description for the useEffect.

src/components/CompareResults/ResultsTable.tsx Outdated Show resolved Hide resolved
@ST-KO ST-KO force-pushed the feature/persist-column-filters-in-url branch from 4794bc3 to ddf1de9 Compare October 22, 2024 14:06
Copy link

codecov bot commented Oct 22, 2024

Codecov Report

Attention: Patch coverage is 94.59459% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.57%. Comparing base (ccff23e) to head (69fb47d).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/hooks/useTableFilters.ts 94.28% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #788   +/-   ##
=======================================
  Coverage   91.56%   91.57%           
=======================================
  Files          89       90    +1     
  Lines        2359     2385   +26     
  Branches      443      447    +4     
=======================================
+ Hits         2160     2184   +24     
- Misses        176      178    +2     
  Partials       23       23           

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

@Carla-Moz
Copy link
Contributor

@ST-KO please be sure to keep your branch updated :)

Also, 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

@Carla-Moz Carla-Moz self-requested a review October 24, 2024 19:05
@ST-KO
Copy link
Author

ST-KO commented Oct 24, 2024

@ST-KO please be sure to keep your branch updated :)

Also, 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

@Carla-Moz Thank you. :)

I have updated my feature branch (ST-KO:feature/persist-column-filters-in-url) and as of now, it's up to date with the latest changes.

@ST-KO ST-KO force-pushed the feature/persist-column-filters-in-url branch from ad14c2c to 69fb47d Compare October 24, 2024 23:51
@Carla-Moz
Copy link
Contributor

@ST-KO Awesome!

Also, please be sure to submit your final application before the Oct 29th deadline or you won't be considered for the intern selection. https://www.outreachy.org/docs/applicant/#final-application

@ST-KO
Copy link
Author

ST-KO commented Oct 29, 2024

@ST-KO Awesome!

Also, please be sure to submit your final application before the Oct 29th deadline or you won't be considered for the intern selection. https://www.outreachy.org/docs/applicant/#final-application

Hi @Carla-Moz,

Thank you for your reminder. I've just submitted my final application. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants