mozilla / perfcompare

Improved Performance Comparison Tool
Mozilla Public License 2.0
39 stars 93 forks source link

Remove suspense comps and add lazy import for results table #698

Closed Carla-Moz closed 2 months ago

Carla-Moz commented 2 months ago

This commit removes the Suspense component since it doesn't fulfill our task of giving users feedback during the edit work flow. The current implementation requires reloading the page which isn't ideal and honestly, unnecessary. Therefore, I've also removed this from the form.

My proposed solution adds an isLoadingResults state to ResultView to manage the rendering of the spinner upon initial loading of the ResultsTable and during any revision updates. In this commit, I implement my fix only on the CompareWithBase component. The form in this component updates isLoadingResults to true until the results are ready. You'll see I added a delay in ResultsView since the results resolve too quickly for users to see any feedback.

If the proposed fix is satisfactory upon review, I'll create a new commit to update CompareOverTime workflow. Lastly, I've imported the ResultTable in ResultsMain using the lazy API to help improve performance. Profile for beta for initial loading of results table (browsertime framework) / Profile for my deployed PR '' ''

My patch reduces the first major jank by 3 seconds.

netlify[bot] commented 2 months ago

Deploy Preview for mozilla-perfcompare ready!

Name Link
Latest commit c90c84a29ebb80ced17983af52b4e20f4c49d443
Latest deploy log https://app.netlify.com/sites/mozilla-perfcompare/deploys/66a841cfd35c620008f9d9e6
Deploy Preview https://deploy-preview-698--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.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.44%. Comparing base (d8a7420) to head (c90c84a). Report is 2 commits behind head on beta.

Files Patch % Lines
src/components/CompareResults/ResultsView.tsx 88.88% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## beta #698 +/- ## ======================================= Coverage 88.43% 88.44% ======================================= Files 75 75 Lines 1894 1904 +10 Branches 350 353 +3 ======================================= + Hits 1675 1684 +9 - Misses 185 186 +1 Partials 34 34 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Carla-Moz commented 2 months ago

No need to review until I add the fix to frameworks dropdown move because this might end up becoming irrelevant.