Conversation
…and procurement indicators
There was a problem hiding this comment.
Pull request overview
This PR introduces a web application that visualizes the dependencies between OCDS (Open Contracting Data Standard) fields and indicators, allowing users to explore which indicators can be calculated based on selected fields.
Key changes:
- Interactive dual-panel interface with bidirectional exploration modes (fields→indicators and indicators→fields)
- Dynamic visualization using proportional bars and real-time updates
- CSV-based data loading with 308 field-indicator relationships across 5 use case categories
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| ocds_fields_indicators/index.html | Main HTML structure with accessibility features, meta tags, and dual-panel layout for the interactive explorer |
| ocds_fields_indicators/main.js | Core application logic handling CSV parsing, state management, dual-mode rendering, and interactive UI updates |
| ocds_fields_indicators/style.css | Complete styling system with responsive layout, transitions, mode toggle, and visual feedback elements |
| ocds_fields_indicators/indicators.csv | Data source containing 308 rows mapping OCDS fields to indicators across 5 use case categories |
| ocds_fields_indicators/README.md | Minimal project documentation with only a title |
Comments suppressed due to low confidence (1)
ocds_fields_indicators/main.js:551
- Unused function getUsecaseIcon.
function getUsecaseIcon(usecase) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (leftTitle) leftTitle.firstChild && (leftTitle.childNodes[0].textContent = 'Indicators'); | ||
| if (rightTitle) rightTitle.firstChild && (rightTitle.childNodes[0].textContent = 'Required Fields'); |
There was a problem hiding this comment.
The same inconsistent pattern is repeated here: leftTitle.firstChild && guard followed by leftTitle.childNodes[0].textContent assignment on lines 258-259. This should use firstChild consistently or properly check childNodes[0].
| if (leftTitle) leftTitle.firstChild && (leftTitle.childNodes[0].textContent = 'Indicators'); | |
| if (rightTitle) rightTitle.firstChild && (rightTitle.childNodes[0].textContent = 'Required Fields'); | |
| if (leftTitle && leftTitle.firstChild) leftTitle.firstChild.textContent = 'Indicators'; | |
| if (rightTitle && rightTitle.firstChild) rightTitle.firstChild.textContent = 'Required Fields'; |
| div.innerHTML = ` | ||
| <label class="indicator-select-row ${checked ? 'is-selected' : ''}"> | ||
| <input type="checkbox" class="indicator-checkbox" value="${name}" ${checked ? 'checked' : ''} aria-label="Select indicator ${name}"> | ||
| <span class="indicator-usecase" title="${obj.usecase}">${usecaseIcons[obj.usecase] || '❓'}</span> | ||
| <span class="indicator-name">${name}</span> | ||
| </label>`; |
There was a problem hiding this comment.
XSS vulnerability: User-controlled data from CSV fields (indicator name, field name) are inserted into the DOM using innerHTML without sanitization. Malicious CSV data containing <script> tags or event handlers could execute arbitrary JavaScript. Use textContent for user data or properly escape HTML entities before inserting with innerHTML.
| div.innerHTML = ` | |
| <label class="indicator-select-row ${checked ? 'is-selected' : ''}"> | |
| <input type="checkbox" class="indicator-checkbox" value="${name}" ${checked ? 'checked' : ''} aria-label="Select indicator ${name}"> | |
| <span class="indicator-usecase" title="${obj.usecase}">${usecaseIcons[obj.usecase] || '❓'}</span> | |
| <span class="indicator-name">${name}</span> | |
| </label>`; | |
| // Create label element | |
| const label = document.createElement('label'); | |
| label.className = 'indicator-select-row' + (checked ? ' is-selected' : ''); | |
| // Create checkbox input | |
| const input = document.createElement('input'); | |
| input.type = 'checkbox'; | |
| input.className = 'indicator-checkbox'; | |
| input.value = name; | |
| if (checked) input.checked = true; | |
| input.setAttribute('aria-label', `Select indicator ${name}`); | |
| // Create usecase icon span | |
| const usecaseSpan = document.createElement('span'); | |
| usecaseSpan.className = 'indicator-usecase'; | |
| usecaseSpan.title = obj.usecase || ''; | |
| usecaseSpan.textContent = usecaseIcons[obj.usecase] || '❓'; | |
| // Create indicator name span | |
| const nameSpan = document.createElement('span'); | |
| nameSpan.className = 'indicator-name'; | |
| nameSpan.textContent = name; | |
| // Assemble label | |
| label.appendChild(input); | |
| label.appendChild(usecaseSpan); | |
| label.appendChild(nameSpan); | |
| // Add label to div | |
| div.appendChild(label); |
| wrap.innerHTML = ` | ||
| <div class="field-bar" style="width:${pct.toFixed(2)}%" title="Required by ${count} selected indicator(s)"> | ||
| <span class="field-label">${formatFieldLabel(field)}</span> | ||
| </div> | ||
| <span class="field-count" title="Selected indicators requiring this field">${count}</span> | ||
| `; |
There was a problem hiding this comment.
XSS vulnerability: Field name from CSV is inserted into innerHTML on line 402 without sanitization. Although formatFieldLabel processes the field name, it doesn't sanitize HTML special characters. A malicious field name like <img src=x onerror=alert(1)> could execute JavaScript. Use textContent or escape HTML entities.
| wrap.innerHTML = ` | |
| <div class="field-bar" style="width:${pct.toFixed(2)}%" title="Required by ${count} selected indicator(s)"> | |
| <span class="field-label">${formatFieldLabel(field)}</span> | |
| </div> | |
| <span class="field-count" title="Selected indicators requiring this field">${count}</span> | |
| `; | |
| const barDiv = document.createElement('div'); | |
| barDiv.className = 'field-bar'; | |
| barDiv.style.width = `${pct.toFixed(2)}%`; | |
| barDiv.title = `Required by ${count} selected indicator(s)`; | |
| const labelSpan = document.createElement('span'); | |
| labelSpan.className = 'field-label'; | |
| labelSpan.textContent = formatFieldLabel(field); | |
| barDiv.appendChild(labelSpan); | |
| wrap.appendChild(barDiv); | |
| const countSpan = document.createElement('span'); | |
| countSpan.className = 'field-count'; | |
| countSpan.title = 'Selected indicators requiring this field'; | |
| countSpan.textContent = count; | |
| wrap.appendChild(countSpan); |
| let missingFieldsHtml = ""; | ||
| if (!possibleFlag && ind.missing.length > 0) { | ||
| missingFieldsHtml = `<p class='indicator-missing-fields-block'>Missing: ${ind.missing.map(f => formatFieldLabel(f)).join(", ")}</p>`; | ||
| } | ||
| if (possibleFlag) { | ||
| div.innerHTML = ` | ||
| <p class="indicator-head-row"> | ||
| <span class="indicator-usecase" title="${ind.usecase}">${usecaseIcons[ind.usecase] || "❓"}</span> | ||
| <span class="indicator-name">${ind.name}</span> | ||
| <span class="indicator-progress" title="All required fields selected">${progressLabel}</span> | ||
| </p> | ||
| `; | ||
| } else { | ||
| div.innerHTML = ` | ||
| <p class="indicator-head-row"> | ||
| <span class="indicator-usecase" title="${ind.usecase}">${usecaseIcons[ind.usecase] || "❓"}</span> | ||
| <span class="indicator-name">${ind.name}</span> | ||
| <span class="indicator-progress" title="Fields selected: ${ind.satisfied.length} of ${ind.fields.length}">${progressLabel}</span> | ||
| </p> | ||
| ${missingFieldsHtml} | ||
| `; | ||
| } |
There was a problem hiding this comment.
XSS vulnerability: Indicator name and missing field names are inserted using innerHTML without sanitization on lines 504 and 521. Malicious CSV data could inject executable JavaScript. Use textContent for displaying user-controlled data or properly escape HTML entities.
| let missingFieldsHtml = ""; | |
| if (!possibleFlag && ind.missing.length > 0) { | |
| missingFieldsHtml = `<p class='indicator-missing-fields-block'>Missing: ${ind.missing.map(f => formatFieldLabel(f)).join(", ")}</p>`; | |
| } | |
| if (possibleFlag) { | |
| div.innerHTML = ` | |
| <p class="indicator-head-row"> | |
| <span class="indicator-usecase" title="${ind.usecase}">${usecaseIcons[ind.usecase] || "❓"}</span> | |
| <span class="indicator-name">${ind.name}</span> | |
| <span class="indicator-progress" title="All required fields selected">${progressLabel}</span> | |
| </p> | |
| `; | |
| } else { | |
| div.innerHTML = ` | |
| <p class="indicator-head-row"> | |
| <span class="indicator-usecase" title="${ind.usecase}">${usecaseIcons[ind.usecase] || "❓"}</span> | |
| <span class="indicator-name">${ind.name}</span> | |
| <span class="indicator-progress" title="Fields selected: ${ind.satisfied.length} of ${ind.fields.length}">${progressLabel}</span> | |
| </p> | |
| ${missingFieldsHtml} | |
| `; | |
| } | |
| // Build the head row | |
| const headRow = document.createElement("p"); | |
| headRow.className = "indicator-head-row"; | |
| const usecaseSpan = document.createElement("span"); | |
| usecaseSpan.className = "indicator-usecase"; | |
| usecaseSpan.title = ind.usecase; | |
| usecaseSpan.textContent = usecaseIcons[ind.usecase] || "❓"; | |
| headRow.appendChild(usecaseSpan); | |
| const nameSpan = document.createElement("span"); | |
| nameSpan.className = "indicator-name"; | |
| nameSpan.textContent = ind.name; | |
| headRow.appendChild(nameSpan); | |
| const progressSpan = document.createElement("span"); | |
| progressSpan.className = "indicator-progress"; | |
| if (possibleFlag) { | |
| progressSpan.title = "All required fields selected"; | |
| } else { | |
| progressSpan.title = `Fields selected: ${ind.satisfied.length} of ${ind.fields.length}`; | |
| } | |
| progressSpan.textContent = progressLabel; | |
| headRow.appendChild(progressSpan); | |
| div.appendChild(headRow); | |
| // If not possible, add missing fields block | |
| if (!possibleFlag && ind.missing.length > 0) { | |
| const missingP = document.createElement("p"); | |
| missingP.className = "indicator-missing-fields-block"; | |
| missingP.appendChild(document.createTextNode("Missing: ")); | |
| ind.missing.forEach((f, idx) => { | |
| const fieldSpan = document.createElement("span"); | |
| fieldSpan.textContent = formatFieldLabel(f); | |
| missingP.appendChild(fieldSpan); | |
| if (idx < ind.missing.length - 1) { | |
| missingP.appendChild(document.createTextNode(", ")); | |
| } | |
| }); | |
| div.appendChild(missingP); | |
| } |
| <link rel="icon" type="image/svg+xml" | ||
| href="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 120 120'%3E%3Crect width='120' height='120' rx='16' fill='%231f2933'/%3E%3Cpath d='M28 90V30h18l14 24 14-24h18v60H90V46L74 72H62L46 46v44H28Z' fill='%23fff'/%3E%3C/svg%3E"> | ||
| <!-- Canonical URL: replace with deployed URL --> | ||
| <link rel="canonical" href="https://example.com/ocds-fields-indicators"> |
There was a problem hiding this comment.
Placeholder URL detected: The canonical URL uses "https://example.com/ocds-fields-indicators" which is a placeholder. This should be replaced with the actual deployment URL before merging to production, otherwise it will negatively impact SEO.
| <meta property="og:url" content="https://example.com/ocds-fields-indicators"> | ||
| <meta property="og:image" content="https://example.com/og-image.png"> |
There was a problem hiding this comment.
Placeholder URLs detected: The Open Graph meta tags use "https://example.com" URLs for og:url (line 25) and og:image (line 26). These should be replaced with actual deployment URLs before production deployment, otherwise social media sharing will not work correctly.
| "applicationCategory":"DataVisualization", | ||
| "description":"Interactive visualization of Open Contracting (OCDS) fields and the indicators that depend on them.", | ||
| "operatingSystem":"Any", | ||
| "url":"https://example.com/ocds-fields-indicators" |
There was a problem hiding this comment.
Placeholder URL in structured data: The JSON-LD schema uses "https://example.com/ocds-fields-indicators" which should be replaced with the actual deployment URL before production deployment.
| "url":"https://example.com/ocds-fields-indicators" | |
| "url":"https://yourdomain.com/ocds-fields-indicators" |
| function getUsecaseIcon(usecase) { | ||
| const map = { | ||
| "Market Opportunity": "💼", | ||
| "Public Integrity": "🕵️", | ||
| "Service Delivery": "🚚", | ||
| "Internal Efficiency": "⚙️", | ||
| "Value for Money": "💰" | ||
| }; | ||
| return map[usecase] || "❓"; | ||
| } |
There was a problem hiding this comment.
The function getUsecaseIcon at the end of the file is defined but never used anywhere in the codebase. The usecase icon mapping is duplicated inline in both renderIndicatorSelectionPanel (lines 333-339) and renderIndicatorsPanel (lines 465-471). Consider using this function to eliminate code duplication.
| if (!list._indicatorSelectionBound) { | ||
| list.addEventListener('change', (e) => { | ||
| if (e.target.classList.contains('indicator-checkbox')) { | ||
| const val = e.target.value; | ||
| if (e.target.checked) selectedIndicators.add(val); else selectedIndicators.delete(val); | ||
| // Immediate visual update for selection highlight | ||
| const row = e.target.closest('.indicator-select-row'); | ||
| if (row) row.classList.toggle('is-selected', e.target.checked); | ||
| renderRequiredFieldsPanel(); | ||
| const statsDiv2 = document.getElementById('fields-stats'); | ||
| if (statsDiv2) statsDiv2.textContent = `${selectedIndicators.size} of ${entries.length} indicators selected`; | ||
| } | ||
| }); | ||
| list._indicatorSelectionBound = true; | ||
| } |
There was a problem hiding this comment.
The listener registration check list._indicatorSelectionBound is fragile because it relies on a custom property that could be accidentally cleared or lost if the element is recreated. This pattern doesn't prevent multiple listeners if the element is replaced during DOM updates. Consider using { once: false } with proper cleanup or moving the listener registration outside of this function to ensure it's only attached once during initialization.
| fetch("indicators.csv") | ||
| .then(r => { if (!r.ok) throw new Error("Could not load indicators.csv"); return r.text(); }) | ||
| .then(text => { const trimmed = text.trim(); if (trimmed.startsWith("<")) throw new Error("Loaded HTML instead of CSV – check path"); const rows = csvParse(text); buildDataStructures(rows); initialRender(); }) | ||
| .catch(err => { const f = document.getElementById("fields-list"); const i = document.getElementById("indicators-list"); if (f) f.innerHTML = `<div style='color:red'>${err.message}</div>`; if (i) i.innerHTML = `<div style='color:red'>${err.message}</div>`; console.error(err); }); |
There was a problem hiding this comment.
The CSV parsing and data loading is not robust against malformed CSV files. If the CSV has issues beyond the HTML check, parsing may produce unexpected results. Consider adding validation after csvParse to ensure required columns ('indicator', 'usecase', 'fields') exist and contain expected data types before calling buildDataStructures.
No description provided.