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

Implement sorting on the main results page #838

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

julienw
Copy link
Contributor

@julienw julienw commented Jan 7, 2025

This implements the sorting feature on the main results page.

In the main results page the results are then processed and organized in a nested structure test -> revision -> platform -> result item. Therefore I was initially afraid that the sorting logic would need to be specially tailored. But instead I got the idea of doing the sort before the processing, and this seems to work just fine.

deploy preview

Here is the list of commits:

  • First commit is Extract the table filtering code to a common file #839, do not review.
  • 2-4 are implementing the bulk of the functionality.
  • 5 cleans up the fixtures file used in the tests
  • 6 improves summarizeVisibleRows so that the delta value is shown can be assessed in the tests, for an easier readability in the test cases.
  • 7 adds new tests for the new feature
  • 8 does some refactoring

Copy link

netlify bot commented Jan 7, 2025

Deploy Preview for mozilla-perfcompare ready!

Name Link
🔨 Latest commit 83718c5
🔍 Latest deploy log https://app.netlify.com/sites/mozilla-perfcompare/deploys/678694f5ccad460008f77bac
😎 Deploy Preview https://deploy-preview-838--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 Jan 7, 2025

Codecov Report

Attention: Patch coverage is 93.47826% with 3 lines in your changes missing coverage. Please review.

Project coverage is 93.38%. Comparing base (be87317) to head (83718c5).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/hooks/useTableSort.ts 82.35% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #838      +/-   ##
==========================================
+ Coverage   93.20%   93.38%   +0.17%     
==========================================
  Files          93       93              
  Lines        2562     2584      +22     
  Branches      508      514       +6     
==========================================
+ Hits         2388     2413      +25     
+ Misses        155      152       -3     
  Partials       19       19              

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

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.

This looks great to me. Nothing to add here. Thanks!

@julienw julienw force-pushed the sort-on-main-results-page branch 2 times, most recently from 03af01f to 0a070f5 Compare January 9, 2025 11:05
@julienw
Copy link
Contributor Author

julienw commented Jan 9, 2025

@Carla-Moz you already reviewed commits 2-4. I haven't touched them besides a rebase and conflicts, but can you please a look at commits 5 to 8 ? (Commit 1 is #839 to be reviewed indepently).

Thanks!

@julienw julienw requested a review from Carla-Moz January 9, 2025 13:42
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.

Tested and looks ready to ship to me. Thank you for making it easy to follow.

@julienw julienw force-pushed the sort-on-main-results-page branch from 587cdd0 to 83718c5 Compare January 14, 2025 16:46
@julienw julienw merged commit 5634e3c into mozilla:main Jan 14, 2025
8 checks passed
@julienw julienw mentioned this pull request Jan 15, 2025
julienw added a commit that referenced this pull request Jan 15, 2025
Updates:

[Julien Wajsberg] Small adjustments on the sort functionality (#837)
[esanuandra] PCF-562 Consider adding the OS version to the name of the platform in the "Platform" column (#816)
[Carla Severe] Removed repetitive style references (#836)
[Julien Wajsberg] Rename OSX to macOS to better reflect the newest naming for this platform (#840)
[Julien Wajsberg] Extract the table filtering code to a common file (#839)
[Julien Wajsberg] Implement sorting on the main results page (#838)
[Chineta Adinnu] Add tooltips to revision column headers (#832)
[Chineta Adinnu] Add aria expanded to revision row expand button (#826)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants