Add ability to optionally specify two additional segments

[tunetheweb]

This adds the ability to optionally set 2 more segments when using using Custom segment selection.

When using any of the pre-defined segments, there is no difference.

By default these are set to None (which are only available in these third and fourth options, as first two are mandatory):

This useful when comparing segments:

Highcharts automatically gives them slightly different labels:

And the table, now shows these segments:

The histograms are less clear, but you can hover over the labels to see them:

[tunetheweb]

Any thoughts on this @philipwalton ?

[philipwalton]

Hey @tunetheweb, sorry I originally got this when I was on vacation, and I've then have been busy with I/O stuff since getting back and forgot to reply to this.

Any thoughts on this @philipwalton ?

Yes, lots of thoughts :)

I don't have any particular objection to this change in general. I do have some concerns that the more segments you put into one of these charts, the harder it is to see what's going on. And you can already get the data this feature adds from the current version—you just have to run the report a few times (though you don't see everything together).

Stepping back a bit, I've also been trying to balance the purpose of this tool, which is somewhere in between 1) it being useful as a tool itself, and 2) being a reference that developers/teams can build on top of. The original goal was to add just enough of (1) that it encourage teams, and perhaps even the ecosystem, to do (2) but I haven't seen much of (2) happening (outside of your work, TBH).

Another consideration is the fact that Universal Analytics is being deprecated next summer, so at some point in the next year this tool will either have to be deprecated too or be updated to support GA4. I'm in conversations with one of our engineering teams to see if they want to take over ownership and build this into more of a project, but that's still early stages.

WDYT?

At minimum, if we do add this feature I'd think it'd be good to update the colors used for the 3rd and 4th segment in the charts so that it's different from the other. Right now it looks like the colors are reused.

[tunetheweb]

I don't have any particular objection to this change in general. I do have some concerns that the more segments you put into one of these charts, the harder it is to see what's going on.

I agree, and think most will continue use two segments. But there's a few cases (e.g. DeviceMemory, or Browsers - if/when they start to support CWV more) where more segments can be useful. And given the code changes are not too messy IMHO, and the work is done, I think it could be useful to merge this.

I do agree some of the charts can be difficult to read, depending on the data, but the hovering over the labels to highlight particular segments helps with that. And if people find it too messy for certain queries, they can always drop back to just using the two.

And you can already get the data this feature adds from the current version—you just have to run the report a few times (though you don't see everything together).

Yeah but the ability to see them together is the point.

Stepping back a bit, I've also been trying to balance the purpose of this tool, which is somewhere in between 1) it being useful as a tool itself, and 2) being a reference that developers/teams can build on top of. The original goal was to add just enough of (1) that it encourage teams, and perhaps even the ecosystem, to do (2) but I haven't seen much of (2) happening (outside of your work, TBH).

I think I heard you mention that before!

Personally I think it's a useful tool. I am not sure if people will build on top of it (there's many other tools to extract data from GA), and history has shown that to be the case as you say. So it seems to me, it's either use this tool, or use an alternative. Personally, as with any open source software, I would also prefer that if people do expand it with something that might be useful to others and it's not too messy an addition to code and maintain going forward, then they submit PRs (like this one), to upgrade the original and benefit others. Even if people have forked it, feeding back to upstream is a good thing to do IMHO. That's not to say the original project needs to accept any PRs if they don't think it's useful to be in the original.

But if it's not something you want to maintain and it was purely meant as a proof of concept, then that's understandable too. But the README should be updated to clarify that IMHO.

Another consideration is the fact that Universal Analytics is being deprecated next summer, so at some point in the next year this tool will either have to be deprecated too or be updated to support GA4. I'm in conversations with one of our engineering teams to see if they want to take over ownership and build this into more of a project, but that's still early stages.

That seems orthogonal to this change. Obviously if that work is not done then need to retire the tool. But a year is a long time away (and personally I think that timeline is aggressive given GA4 rollout and suspect it might even be pushed out, but that's a pure guess from my side and sure you know more than me!).

I can understand not wanting to invest in adding features until that's more certain, but since the investment is mostly done (unless you see the need for lots of changes?) then think it's worthwhile to merge.

At minimum, if we do add this feature I'd think it'd be good to update the colors used for the 3rd and 4th segment in the charts so that it's different from the other. Right now it looks like the colors are reused.

Oh I hadn't even realised the colors were set and thought those were the highchart defaults (like the symbol on the line!). Added some new colors in https://github.com/GoogleChromeLabs/web-vitals-report/pull/18/commits/fdd61373aa995a1512efc731bdba4cf5d97085c5. Picking colors is always a pain, but tried to choose ones similar to the previous ones, rather than completely different ones (e.g. purple)? Maybe would be better to differentiate more. Let me know your thoughts on this.

But also happy to close this if you feel it's more hassle than it's worth, or the project might be deprecated soon. I did this for other reasons and found it easier to do in this tool, and after that spent a (very small!) bit of time cleaning it up to submit the PR. But won't be offended if you'd prefer not to merge and close this instead.