Add new usage chart to the feature detail page #944
Conversation
|
The existing browsers used the name "Chrome", and so I stuck with that over Chromium. If that is not what we want, I can make a change to this PR. |
There was a problem hiding this comment.
One question about handleUsageBrowserSelection below.
Also, since we still need to correct the usage data, we should hide the chart by default. Then we could enable it with something like a query parameter (we need a feature flag capability like WPT.fyi). That will influence the e2e test. (You will need to have a temporary case that passes the flag).
Other notes:
Also, from an organization perspective, we should probably create two new components: webstatus-feature-page-feature-support-chart and webstatus-feature-page-feature-usage-chart. They can both subclass webstatus-gchart.
For example in webstatus-feature-page-feature-usage-chart, you could put all of the new @State variables, the new loading task into it, createFeatureUsageDataFromMap, and other usage specific things into that component. And do the same thing for the other chart. This will help encapsulate specific logic and settings for each chart into each specialized component
The organization part can come as a follow up PR.
|
I have hidden the chart behind a query parameter here, but I have not made the organizational changes, as that was suggested for a separate PR. 🙂 |
There was a problem hiding this comment.
I added notes for more refactoring (and some unit tests) for the organization follow up PR.
Otherwise, the only other thing I would change is this PR would be changing getChromiumDailyUsageStats to return AsyncIterable. (which will cause _fetchFeatureUsageData to change slightly)
|
I added all the comments to #964 to track for a follow-up refactoring PR |
| @@ -37,6 +37,26 @@ test('matches the screenshot', async ({page}) => { | |||
| await expect(pageContainer).toHaveScreenshot(); | |||
| }); | |||
|
|
|||
| // TODO: Remove this test once the usage chart is enabled by default. | |||
| test('matches the screenshot with usage chart', async ({page}) => { | |||
| await page.goto('http://localhost:5555/features/odit64?showUsageChart'); | |||
There was a problem hiding this comment.
Nit: use hyphens to separate the words: show-usage-chart [1]
[1] https://developers.google.com/search/docs/crawling-indexing/url-structure
Fixes #885
This change adds a new "Feature Usage" chart to the feature detail page in the same design as the Feature Support chart.
Note: this PR is built on top of the changes found in #941 and will display them as new changes until that PR has been landed.