Skip to content

Commit bbcfe47

Browse files
committed
Address PR review feedback
- Rename title/description per suggestion ("Core Web Vitals histograms") - Restructure button bar so all 3 buttons appear first in DOM order, followed by content wrappers (fixes tab order) - Remove per-component metric selectors; the pass-rate timeseries selector now drives the geo breakdown and histogram via a shared cwv-metric-change event - Deep-linking works via ?good-cwv-over-time=CLS - Histogram resolves "overall" to LCP (has no combined view) - Titles update dynamically ("LCP histogram", "INP geographic breakdown") - Rename "Show distribution" button to "Show histogram"
1 parent 6c93746 commit bbcfe47

File tree

8 files changed

+66
-91
lines changed

8 files changed

+66
-91
lines changed

config/techreport.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -737,8 +737,8 @@
737737
},
738738
"cwv_distribution": {
739739
"id": "cwv_distribution",
740-
"title": "Core Web Vitals distribution",
741-
"description": "How origins are distributed across performance buckets for individual Core Web Vitals metrics. Green, orange, and red zones indicate good, needs improvement, and poor thresholds respectively.",
740+
"title": "Core Web Vitals histograms",
741+
"description": "How origins are distributed for individual Core Web Vitals metrics. Green, orange, and red zones indicate good, needs improvement, and poor thresholds respectively.",
742742
"metric_options": [
743743
{ "label": "LCP", "value": "LCP" },
744744
{ "label": "CLS", "value": "CLS" },

src/js/techreport/cwvDistribution.js

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,16 @@ class CwvDistribution {
3030
this.id = id;
3131
this.pageFilters = filters;
3232
this.distributionData = null;
33-
this.selectedMetric = 'LCP';
3433
this.chart = null;
3534
this.root = document.querySelector(`[data-id="${this.id}"]`);
3635
this.date = this.pageFilters.end || this.root?.dataset?.latestDate || '';
36+
this.selectedMetric = this.resolveMetric(new URLSearchParams(window.location.search).get('good-cwv-over-time'));
3737

3838
// Populate "Latest data" timestamp immediately
3939
const tsSlot = this.root?.querySelector('[data-slot="cwv-distribution-timestamp"]');
4040
if (tsSlot && this.date) tsSlot.textContent = UIUtils.printMonthYear(this.date);
4141

42-
42+
this.updateTitle();
4343
this.bindEventListeners();
4444

4545
// Auto-expand if URL hash targets this section
@@ -48,14 +48,27 @@ class CwvDistribution {
4848
}
4949
}
5050

51+
// Map the shared metric value (which may be 'overall') to a metric this chart can show
52+
resolveMetric(value) {
53+
if (value && METRIC_CONFIG[value]) return value;
54+
return 'LCP';
55+
}
56+
57+
updateTitle() {
58+
const slot = this.root?.querySelector('[data-slot="cwv-distribution-title-metric"]');
59+
if (slot) slot.textContent = this.selectedMetric;
60+
}
61+
5162
bindEventListeners() {
5263
if (!this.root) return;
5364

54-
this.root.querySelectorAll('.cwv-distribution-metric-selector').forEach(dropdown => {
55-
dropdown.addEventListener('change', event => {
56-
this.selectedMetric = event.target.value;
65+
document.addEventListener('cwv-metric-change', (event) => {
66+
const resolved = this.resolveMetric(event.detail?.value);
67+
if (resolved !== this.selectedMetric) {
68+
this.selectedMetric = resolved;
69+
this.updateTitle();
5770
if (this.distributionData) this.renderChart();
58-
});
71+
}
5972
});
6073

6174
const btn = document.getElementById('cwv-distribution-btn');
@@ -71,15 +84,15 @@ class CwvDistribution {
7184
const btn = document.getElementById('cwv-distribution-btn');
7285
if (show) {
7386
this.root.classList.remove('hidden');
74-
if (btn) btn.textContent = 'Hide distribution';
87+
if (btn) btn.textContent = 'Hide histogram';
7588
if (!this.distributionData) {
7689
this.fetchData();
7790
} else if (this.chart) {
7891
this.chart.reflow();
7992
}
8093
} else {
8194
this.root.classList.add('hidden');
82-
if (btn) btn.textContent = 'Show distribution';
95+
if (btn) btn.textContent = 'Show histogram';
8396
}
8497
}
8598

src/js/techreport/geoBreakdown.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,12 @@ class GeoBreakdown {
99
this.pageFilters = filters;
1010
this.data = data;
1111
this.geoData = null;
12-
this.selectedMetric = 'overall';
12+
this.selectedMetric = new URLSearchParams(window.location.search).get('good-cwv-over-time') || 'overall';
1313
this.sortColumn = 'total';
1414
this.sortDir = 'desc';
1515
this.showAll = false;
1616

17+
this.updateTitle();
1718
this.bindEventListeners();
1819

1920
// Auto-expand if URL hash targets this section
@@ -37,13 +38,20 @@ class GeoBreakdown {
3738
}
3839
}
3940

41+
updateTitle() {
42+
const slot = document.querySelector('[data-slot="geo-title-metric"]');
43+
if (!slot) return;
44+
slot.textContent = this.selectedMetric === 'overall' ? 'Core Web Vitals' : this.selectedMetric;
45+
}
46+
4047
bindEventListeners() {
41-
const root = `[data-id="${this.id}"]`;
42-
document.querySelectorAll(`${root} .geo-metric-selector`).forEach(dropdown => {
43-
dropdown.addEventListener('change', event => {
44-
this.selectedMetric = event.target.value;
48+
document.addEventListener('cwv-metric-change', (event) => {
49+
const value = event.detail?.value;
50+
if (value && value !== this.selectedMetric) {
51+
this.selectedMetric = value;
52+
this.updateTitle();
4553
if (this.geoData) this.renderTable();
46-
});
54+
}
4755
});
4856

4957
const btn = document.getElementById('geo-breakdown-btn');

src/js/techreport/timeseries.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ class Timeseries {
4646
// Re-render the content
4747
this.updateContent();
4848
this.updateInfo(metric, endpoint);
49+
50+
// Broadcast metric change so linked components (geo breakdown, distribution) update
51+
if (event.target.dataset.param === 'good-cwv-over-time') {
52+
document.dispatchEvent(new CustomEvent('cwv-metric-change', { detail: { value: metric } }));
53+
}
4954
}
5055
}
5156

static/css/techreport/techreport.css

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2355,12 +2355,6 @@ path.highcharts-tick {
23552355
margin-top: 0.5rem;
23562356
}
23572357

2358-
.cwv-button-bar > .geo-breakdown-wrapper,
2359-
.cwv-button-bar > .cwv-distribution-wrapper,
2360-
.cwv-button-bar > [id$="-table-wrapper"] {
2361-
flex-basis: 100%;
2362-
order: 1;
2363-
}
23642358

23652359
/* CWV Distribution histogram */
23662360
.cwv-distribution-wrapper {
@@ -2377,12 +2371,8 @@ path.highcharts-tick {
23772371
overflow-x: auto;
23782372
}
23792373

2380-
.cwv-distribution-header {
2381-
display: flex;
2382-
flex-wrap: wrap;
2383-
justify-content: space-between;
2384-
align-items: flex-start;
2385-
gap: 1rem;
2374+
.cwv-distribution-header h3 {
2375+
margin-top: 0;
23862376
}
23872377

23882378
.cwv-distribution-header .descr {

templates/techreport/components/cwv_distribution.html

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
{% set cwv_distribution_config = tech_report_page.config.cwv_distribution %}
22

3-
<button class="btn show-table cwv-distribution-toggle" type="button" id="cwv-distribution-btn">
4-
Show distribution
5-
</button>
6-
73
<div
84
id="section-{{ cwv_distribution_config.id }}"
95
class="cwv-distribution-wrapper hidden"
@@ -13,31 +9,10 @@
139
data-latest-date="{{ all_dates[0].value if all_dates else '' }}"
1410
>
1511
<div class="cwv-distribution-header">
16-
<div>
17-
<h3><a href="#section-{{ cwv_distribution_config.id }}" class="anchor">{{ cwv_distribution_config.title }}</a></h3>
18-
<p class="descr">{{ cwv_distribution_config.description }}</p>
19-
<h4 class="heading">Latest data: <span data-slot="cwv-distribution-timestamp"></span></h4>
20-
{% include "techreport/components/filter_meta.html" %}
21-
</div>
22-
<div class="component-filters">
23-
<div class="subcategory-selector-wrapper">
24-
<div class="position-wrapper">
25-
<label for="cwv-distribution-metric-selector">Metric</label>
26-
<select
27-
class="cwv-distribution-metric-selector"
28-
id="cwv-distribution-metric-selector"
29-
>
30-
{% for option in cwv_distribution_config.metric_options %}
31-
{% if option.value == 'LCP' %}
32-
<option value="{{ option.value }}" selected>{{ option.label }}</option>
33-
{% else %}
34-
<option value="{{ option.value }}">{{ option.label }}</option>
35-
{% endif %}
36-
{% endfor %}
37-
</select>
38-
</div>
39-
</div>
40-
</div>
12+
<h3><a href="#section-{{ cwv_distribution_config.id }}" class="anchor"><span data-slot="cwv-distribution-title-metric">Core Web Vitals</span> histogram</a></h3>
13+
<p class="descr">{{ cwv_distribution_config.description }}</p>
14+
<h4 class="heading">Latest data: <span data-slot="cwv-distribution-timestamp"></span></h4>
15+
{% include "techreport/components/filter_meta.html" %}
4116
</div>
4217

4318
<div id="{{ cwv_distribution_config.id }}-chart"></div>

templates/techreport/components/geo_breakdown.html

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
{% set geo_breakdown_config = tech_report_page.config.geo_breakdown %}
22

3-
<button class="btn show-table geo-breakdown-toggle" type="button" id="geo-breakdown-btn">
4-
Show geographic breakdown
5-
</button>
6-
73
<div
84
id="section-{{ geo_breakdown_config.id }}"
95
class="geo-breakdown-wrapper hidden"
@@ -13,29 +9,9 @@
139
>
1410
<div class="component-heading-wrapper">
1511
<div class="component-heading">
16-
<h3><a href="#section-{{ geo_breakdown_config.id }}" class="anchor">{{ geo_breakdown_config.title }}</a></h3>
12+
<h3><a href="#section-{{ geo_breakdown_config.id }}" class="anchor"><span data-slot="geo-title-metric">Core Web Vitals</span> geographic breakdown</a></h3>
1713
<p class="descr">{{ geo_breakdown_config.description }}</p>
1814
</div>
19-
20-
<div class="component-filters">
21-
<div class="subcategory-selector-wrapper">
22-
<div class="position-wrapper">
23-
<label for="geo-metric-selector-{{ geo_breakdown_config.id }}">Metric</label>
24-
<select
25-
class="geo-metric-selector"
26-
id="geo-metric-selector-{{ geo_breakdown_config.id }}"
27-
>
28-
{% for option in geo_breakdown_config.metric_options %}
29-
{% if option.value == 'overall' %}
30-
<option value="{{ option.value }}" selected>{{ option.label }}</option>
31-
{% else %}
32-
<option value="{{ option.value }}">{{ option.label }}</option>
33-
{% endif %}
34-
{% endfor %}
35-
</select>
36-
</div>
37-
</div>
38-
</div>
3915
</div>
4016

4117
<div class="geo-breakdown-controls">

templates/techreport/drilldown.html

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,37 +72,45 @@ <h3>{{ tech_report_page.config.good_cwv_summary.title }}</h3>
7272
<div class="cwv-button-bar">
7373
<button class="btn show-table cwv-show-table-btn" type="button" data-id="{{ ts_id }}">Show table</button>
7474
{% if tech_report_page.config.geo_breakdown %}
75-
{% include "techreport/components/geo_breakdown.html" %}
75+
<button class="btn show-table geo-breakdown-toggle" type="button" id="geo-breakdown-btn">Show geographic breakdown</button>
7676
{% endif %}
7777
{% if tech_report_page.config.cwv_distribution %}
78-
{% include "techreport/components/cwv_distribution.html" %}
78+
<button class="btn show-table cwv-distribution-toggle" type="button" id="cwv-distribution-btn">Show histogram</button>
7979
{% endif %}
8080
</div>
81+
82+
{% if tech_report_page.config.geo_breakdown %}
83+
{% include "techreport/components/geo_breakdown.html" %}
84+
{% endif %}
85+
{% if tech_report_page.config.cwv_distribution %}
86+
{% include "techreport/components/cwv_distribution.html" %}
87+
{% endif %}
88+
8189
<script nonce="{{ csp_nonce() }}">
8290
(function() {
8391
var bar = document.querySelector('.cwv-button-bar');
8492
var tw = document.getElementById('{{ ts_id }}-table-wrapper');
8593

86-
// Move table wrapper after the button bar so expanding it doesn't push buttons down
87-
if (bar && tw) bar.after(tw);
94+
// Move the timeseries' table wrapper out of the timeseries div so expanding it
95+
// doesn't push the button bar down
96+
if (bar && tw) bar.parentNode.insertBefore(tw, bar.nextSibling);
8897

8998
// Mutual exclusion: only one panel open at a time
9099
var panels = [
91-
{ btn: bar.querySelector('.cwv-show-table-btn'), wrapper: tw, showText: 'Show table', hideText: 'Hide table' },
92-
{ btn: document.getElementById('geo-breakdown-btn'), wrapper: document.getElementById('section-geo_breakdown'), showText: 'Show geographic breakdown', hideText: 'Hide geographic breakdown' },
93-
{ btn: document.getElementById('cwv-distribution-btn'), wrapper: document.getElementById('section-cwv_distribution'), showText: 'Show distribution', hideText: 'Hide distribution' }
100+
{ btn: bar.querySelector('.cwv-show-table-btn'), wrapper: tw, showText: 'Show table' },
101+
{ btn: document.getElementById('geo-breakdown-btn'), wrapper: document.getElementById('section-geo_breakdown'), showText: 'Show geographic breakdown' },
102+
{ btn: document.getElementById('cwv-distribution-btn'), wrapper: document.getElementById('section-cwv_distribution'), showText: 'Show histogram' }
94103
].filter(function(p) { return p.btn && p.wrapper; });
95104

96105
panels.forEach(function(panel) {
97106
panel.btn.addEventListener('click', function() {
98-
// Close other panels
99107
panels.forEach(function(other) {
100108
if (other !== panel && !other.wrapper.classList.contains('hidden')) {
101109
other.wrapper.classList.add('hidden');
102110
other.btn.textContent = other.showText;
103111
}
104112
});
105-
}, true); // capture phase — runs before component handlers
113+
}, true);
106114
});
107115
})();
108116
</script>

0 commit comments

Comments
 (0)