mozilla / perfcompare

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

remove save btn and move cancel #667

Closed Carla-Moz closed 4 months ago

Carla-Moz commented 4 months ago

Also removed styles related to save cancel pair; updated search component and compare button files; removed staging state since save is no longer needed

Resolved comments guide: The revised PR does the following:

Please see resolved comments commit.

Update: I don't think I made it clear that I would handle the global edit button in a separate PR but I've gone ahead and handle it in this PR based on the comments. I apologize for the extra review.

This PR now closes:

netlify[bot] commented 4 months ago

Deploy Preview for mozilla-perfcompare ready!

Name Link
Latest commit ed0c9a15104e31b318e3141299861cabbdf3ce05
Latest deploy log https://app.netlify.com/sites/mozilla-perfcompare/deploys/6661373b101ee400084651db
Deploy Preview https://deploy-preview-667--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 4 months ago

Codecov Report

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

Project coverage is 91.80%. Comparing base (8dd8ace) to head (ed0c9a1).

Files Patch % Lines
src/components/Search/CompareOverTime.tsx 60.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## beta #667 +/- ## ========================================== - Coverage 91.98% 91.80% -0.19% ========================================== Files 69 68 -1 Lines 1598 1586 -12 Branches 285 285 ========================================== - Hits 1470 1456 -14 - Misses 102 104 +2 Partials 26 26 ```

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

julienw commented 4 months ago

I wasn't sure how ready the PR was but still went through it and gave a few comments.

I thought we agreed that we didn't want to change edit/cancel that much for the MVP (just removing the Save button), but well I'm not opposed.

Carla-Moz commented 4 months ago

I wasn't sure how ready the PR was but still went through it and gave a few comments.

I thought we agreed that we didn't want to change edit/cancel that much for the MVP (just removing the Save button), but well I'm not opposed.

I decided to go ahead and do this because while I was working on the edit feature for CompareOverTime, it didn't make sense to replicate an edit feature that we would revamp so I wanted it out of the way. Both components share many of the same edit comps, config, etc.