Closed julienw closed 4 months ago
Name | Link |
---|---|
Latest commit | 48d49e2c45f0481a60ed5b3927ac72036aa8657a |
Latest deploy log | https://app.netlify.com/sites/mozilla-perfcompare/deploys/665f44a1d50d250008269249 |
Deploy Preview | https://deploy-preview-658--mozilla-perfcompare.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 91.98%. Comparing base (
3a2ba92
) to head (48d49e2
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Rebased!
Thanks for the review!
Deploy preview
You can find 4 commits that can be looked independently (again :D)
Commit 1: very simple refactoring where I move the very simple ResultsHeader into ResultsMain. Commit 2: very simple change adding an aria-label to search inputs to make them easier to target in tests. It's useful for this feature adding another search box, so that the new tests will be able to target the new search box easily. This is also good for accessibility. Commit 3: this implements the search functionality. Commit 4: this adds new tests. Commit 5: remove the apply button.
Tell me what you think!
As I mentioned before, I'm not sure we'll want to keep the Apply button but I still kept it for now.Update: after all I decided to remove it (last commit). I also tweaked the placeholder so that we see more of it (updated the 3rd commit). Esp I removed the "Search by" from the placeholder. Here is the new screenshot: