cloudfour / lighthouse-parade

A Node.js command line tool that crawls a domain and gathers lighthouse performance data for every page.
MIT License
357 stars 14 forks source link

Implement google sheets output writer #119

Closed calebeby closed 2 years ago

calebeby commented 2 years ago

A spreadsheet it made:

https://docs.google.com/spreadsheets/d/1h48etlcEcca1GPPvWZbdFUKZvI-oRU441xVQfusthng/edit#gid=718593030

Try it out!

Make sure to use the -o google-sheets flag to make it upload to google sheets.

Right now the "sign in with google" oauth thing is in "test mode" so I have to manually enter in the email addresses of test users. When you are going to test this, tell me, so I can add your email there.

Eventually we'll need to verify the app with google which will be kinda annoying but it's necessary to make it open to everyone.

changeset-bot[bot] commented 2 years ago

⚠️ No Changeset found

Latest commit: 9f32ba6dffbb55adf65df8a953cb1c30233997e3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

calebeby commented 2 years ago

I assume docs changes come when we merge next into main?

Yes. https://github.com/cloudfour/lighthouse-parade/issues/117

calebeby commented 2 years ago

@Paul-Hebert ready for another review!

calebeby commented 2 years ago

@Paul-Hebert:

I wonder if there's some way to let folks know that it may take a minute for the Google Sheet to populate? I was staring at an empty sheet for a bit which might be confusing.

Do you have ideas for how we could deal with this? Maybe we could write a row that says "waiting for first report" at the top which gets overridden later?

Paul-Hebert commented 2 years ago

I think that's a great idea! If we wanted we could also add a note to the CLI output saying "This may take a while" or something, but I think temporary row you suggested makes the most sense :+1:

gerardo-rodriguez commented 2 years ago

A spreadsheet it made:

https://docs.google.com/spreadsheets/d/1h48etlcEcca1GPPvWZbdFUKZvI-oRU441xVQfusthng/edit#gid=718593030

@calebeby I don't know Google Sheets enough to know how hard this is or if it's even possible, but is it possible to lock the top two rows so that when I sort the sheet I don't get the 2nd header row at the bottom? 🤔

Screen Shot 2022-07-01 at 10 48 14 AM
calebeby commented 2 years ago

Yes, I set up a default filter to use instead of the filter view. If you exit the filter view there will be little icons next to each row header, and you can use those to sort. They will keep the header rows in place.

Unlike a filter view, it does actually modify the order of the data :( but better than nothing I think, and it is nice to have it on the main view instead of requiring people to know where in the menu filter views are.

Screen Shot 2022-07-01 at 10 55 23 AM
gerardo-rodriguez commented 2 years ago

If you exit the filter view there will be little icons next to each row header, and you can use those to sort. They will keep the header rows in place.

@calebeby I don't see the little icons next to each row header. 🤔

Screen Shot 2022-07-01 at 10 59 55 AM
calebeby commented 2 years ago

@gerardo-rodriguez You have to have edit access since it actually modifies the order of the data

calebeby commented 2 years ago

Maybe I can set up a filter view as well as a default filter, so you can make actual edits to the sorting if you have edit access, or just view it sorted differently if you don't have edit access

gerardo-rodriguez commented 2 years ago

@calebeby If I only run it on one page (say a SPA for example) is it expected that a Performance (category score) of 96 be red? 🤔

Screen Shot 2022-07-01 at 1 46 11 PM

https://docs.google.com/spreadsheets/d/1iv-59IP0jdQRNhejsXDX1yot6kJnNh_eFd3QqFznoy8/edit?usp=sharing

calebeby commented 2 years ago

@gerardo-rodriguez It is red because it is the lowest value in the column.

You can try editing the conditional formatting settings in the UI to see if there is a way to make it work well with one row as well as multiple.

Another option is for us to apply conditional formatting only after the 2nd URL is finished, but I do not prefer that option.

Paul-Hebert commented 2 years ago

@gerardo-rodriguez It is red because it is the lowest value in the column.

Oh interesting, I didn't realize that. 🤔 I wonder if it would make more sense to have red, green, and yellow map to "good"/"medium"/"bad" in Lighthouse?

The metrics scores and the perf score are colored according to these ranges:

0 to 49 (red): Poor 50 to 89 (orange): Needs Improvement 90 to 100 (green): Good

I suppose that would only work for the "score" values not the values with units 🤔

calebeby commented 2 years ago

I suppose that would only work for the "score" values not the values with units 🤔

We already have it conditional based on the type of column, because for "score" columns, higher is better, but for the rest (most of which are in milliseconds), lower is better.

We could probably make that work. But I do like to see a continuous range and have it update based on the range of values, because, e.g., on a site like 11ty.dev which gets 100's across the board, a 98 perf score is a much bigger deal than on cloudfour.com

gerardo-rodriguez commented 2 years ago

We could probably make that work. But I do like to see a continuous range and have it update based on the range of values, because, e.g., on a site like 11ty.dev which gets 100's across the board, a 98 perf score is a much bigger deal than on cloudfour.com

A continuous range makes sense based on the range of values. But still, even on the 11ty.dev example, it's arguable a 98 perf score should still be in the green range even if that's the low when all of the rest are 100.

If I were taking a screenshot, I'd want to say "Look at all the green!" and not "Look at all the green...(ignore the red, it's actually good also)".

I don't think it should be a blocker to merging this PR and maybe it's okay to have great scores shown as red. Seems like we could create a GH issue and have further discussion. Maybe even from the community as this tool continues to be used. 👍🏽

Paul-Hebert commented 2 years ago

Yeah, I can see pros and cons to both. I agree it's not a blocker. Maybe in the long run we add a flag to let you choose a formatting option?