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) #783

Closed
wants to merge 5 commits into from

Conversation

ST-KO
Copy link

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

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
Screen.Recording.2024-10-11.at.12.09.41.pm.mov
  • After
dc1650f7-f58e-634f-a372-a774b157e808_custom.mp4
  1. 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
Screen.Recording.2024-10-11.at.12.22.10.pm.mov
  • After
Screen.Recording.2024-10-11.at.3.22.00.am.mov
  1. 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
Screen.Recording.2024-10-11.at.12.25.17.pm.mov
  • After
2ffb5282-addc-cbca-8c6e-7ba903cc918a_custom.mp4

Copy link

netlify bot commented Oct 10, 2024

Deploy Preview for mozilla-perfcompare ready!

Name Link
🔨 Latest commit 1c36cfd
🔍 Latest deploy log https://app.netlify.com/sites/mozilla-perfcompare/deploys/670f16cbb61cc800083d8d08
😎 Deploy Preview https://deploy-preview-783--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.

Copy link

codecov bot commented Oct 16, 2024

Codecov Report

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

Project coverage is 91.44%. Comparing base (3a555e3) to head (da01376).

Files with missing lines Patch % Lines
src/components/CompareResults/ResultsTable.tsx 89.47% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #783      +/-   ##
==========================================
- Coverage   91.46%   91.44%   -0.02%     
==========================================
  Files          88       88              
  Lines        2332     2350      +18     
  Branches      434      438       +4     
==========================================
+ Hits         2133     2149      +16     
- Misses        176      178       +2     
  Partials       23       23              

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

@ST-KO
Copy link
Author

ST-KO commented Oct 16, 2024

Hi, I just wanted to mention that the tests were passing in the first pull request. But after refactoring my code, the tests started failing, even though all tests passed locally in VS Code. I’ll investigate further. Thanks.

@Carla-Moz
Copy link
Contributor

Hi! I'll help you investigate this tomorrow! Thanks for your patience.

@ST-KO
Copy link
Author

ST-KO commented Oct 16, 2024

Hi! I'll help you investigate this tomorrow! Thanks for your patience.

Thank you for your help. I think the easiest and cleanest way would be to create a new branch and submit a fresh pull request after deleting the current one. However, I would like to wait for your review on my existing code and get your permission before proceeding with this approach. 😀

@Carla-Moz
Copy link
Contributor

Carla-Moz commented Oct 16, 2024

@ST-KO Hi there! Thanks for your detailed PR. Before I jump into your code, I highly recommend creating unique descriptions for your commit messages. They are all the same so I have very little idea what to expect in the commits. Could you please go ahead and create a fresh PR with amended commit messages? That would help a lot.

Copy link
Contributor

@Carla-Moz Carla-Moz left a comment

Choose a reason for hiding this comment

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

Please see my comments!

@ST-KO
Copy link
Author

ST-KO commented Oct 17, 2024

@ST-KO Hi there! Thanks for your detailed PR. Before I jump into your code, I highly recommend creating unique descriptions for your commit messages. They are all the same so I have very little idea what to expect in the commits. Could you please go ahead and create a fresh PR with amended commit messages? That would help a lot.

Hi @Carla-Moz,

Thank you for your feedback, and I apologise for not providing clear and unique commit messages earlier. I have now created a fresh pull request #788 for your review. Looking forward to your feedback.

Additionally, please let me know if I should close this pull request. Thank you.

@Carla-Moz Carla-Moz closed this Oct 17, 2024
@Carla-Moz
Copy link
Contributor

Closing due to updated PR #788

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