Skip to content

OpenConceptLab/ocl_issues#2903 | a new user's empty project list explains the first step and offers a sample spreadsheet - #103

Merged
snyaggarwal merged 4 commits into
mainfrom
issues#2903
Oct 16, 2026
Merged

snyaggarwal merged 4 commits into
mainfrom
issues#2903

Conversation

@snyaggarwal

Copy link
Copy Markdown
Contributor

Linked Issue

Closes OpenConceptLab/ocl_issues#2903

Screenshot

Screenshot 2026-10-15 at 15 46 45

…ains the first step and offers a sample spreadsheet

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty state reads well and covers the ticket's three parts. One blocker before merge: the sample file doesn't reach production.

P1: the sample CSV isn't deployed, so the link downloads the app's HTML page. The production image serves only dist/, and the Dockerfile copies public/ files into dist/ one by one (Dockerfile#L45-L55). mapper_sample_lab_tests.csv isn't in that list, so nginx answers /mapper_sample_lab_tests.csv with index.html (try_files $uri /index.html). It works locally because the dev server serves public/ directly. Production's answer for that path today is the same one this branch would get: curl -sI https://map.openconceptlab.org/mapper_sample_lab_tests.csv returns 200, text/html. The fix is one more line in that list. It's outside the diff, so it can't be a one-click suggestion:

RUN cp public/mapper_sample_lab_tests.csv dist/

Merge order with ocl_issues#2907 (no change needed here). Today a header is recognised only when it equals a mapped-field label, so of Code,Name,Specimen only Name is used. Code and Specimen get the red ✗ in Field Configuration, and the specimen value isn't sent to $match. The headers follow ocl_issues#2907's patterns, as the ticket asks (specimen → Property: SYSTEM; code columns stay unmapped without an error). Renaming the column to Property: SYSTEM would teach new users the vocabulary that ticket exists to remove. I'd keep Specimen and land this with or after ocl_issues#2907.

Checked and fine

  • The empty state shows only after both project lists have loaded and both are empty. While they load, the skeleton rows show. The page has no search or filter, so there's no filtered-to-empty case.
  • I rendered it at this commit in en, es, zh and pt-BR, with a stubbed API returning empty lists. All four render with no console warnings. The new button runs the same onNewProjectClick as the header's, so the quota and preview checks still apply. When the file is served, the link downloads the CSV.
  • Lint with CI's command (eslint.js src --ext .jsx,.js) exits 0, npm run test:all passes, and the "Lint and test" check is green.
  • The sample holds 10 common lab tests, each with a code, a name and a specimen, and nothing sensitive.
  • All four locales have both keys.

Notes, no change needed

  • If either project request fails, the list also shows this empty state (before this PR it showed "No projects found."). A returning user would then be invited to start from scratch. Worth a follow-up, not this PR.
  • The OCL design system has no empty-state-with-action component yet: oclweb3's EmptyOverview is a stub that takes only a label. Reusing NoResults here is the right call, and this title, hint, button and link layout is a candidate to add to the design system.
  • One optional pt-BR wording suggestion inline.

Codex's pass is posted as a separate review.

Comment thread src/components/map-projects/MapProjects.jsx Outdated
Comment thread src/i18n/locales/pt-BR/translations.json Outdated

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex review, pass 1 (codex-cli 0.162.1, commit 94f4132)

Verdict: findings. Two P1s, one confirmed as a blocker and one answered as a merge-order condition.

  • F1.1 (P1) MapProjects.jsx:222: the sample download returns HTML in production because the CSV isn't packaged. Response: confirmed; the Dockerfile fix is in the main review.
  • F1.2 (P1) mapper_sample_lab_tests.csv:1: the Specimen header isn't recognised by today's field mapping. Response: true until ocl_issues#2907 lands. The ticket ties the headers to that ticket's patterns, so this is merge order rather than a change here; rated P2.
findings.json
{
  "reviewer": "codex-cli 0.162.1",
  "perspective": "defects",
  "mode": "full",
  "commit": "94f413234ad82479db3262f3947014456c291c77",
  "verdict": "findings",
  "prior": [],
  "findings": [
    {
      "id": "F1.1",
      "severity": "P1",
      "tag": null,
      "file": "src/components/map-projects/MapProjects.jsx",
      "line": 222,
      "claim": "The sample download returns HTML in production because the CSV is not packaged.",
      "scenario": "A user clicks the download link in the deployed Docker build. Webpack copies only src/assets, and Docker's explicit public-file copies omit this CSV. The production image contains only dist, so nginx falls back to index.html for /mapper_sample_lab_tests.csv.",
      "action": "Copy the sample CSV into dist through webpack or the Dockerfile, and verify the production download contains the CSV.",
      "lines_delta": null,
      "confidence": "high"
    },
    {
      "id": "F1.2",
      "severity": "P1",
      "tag": null,
      "file": "public/mapper_sample_lab_tests.csv",
      "line": 1,
      "claim": "The sample's Specimen header is not recognized by the current field mapper.",
      "scenario": "Uploading the sample and selecting LOINC leaves Specimen unmapped, even when the target exposes SYSTEM. isValidColumnValue requires the mapped-field label, and prepareRow consequently drops the specimen: the Glucose row produces only name and synonyms. This fails the recognized-header acceptance criterion; OpenConceptLab/ocl_issues#2907 is not implemented yet.",
      "action": "Use the currently recognized Property: SYSTEM header, or land the corresponding Specimen header-recognition support before shipping the sample.",
      "lines_delta": null,
      "confidence": "high"
    }
  ]
}

@snyaggarwal
snyaggarwal requested a review from paynejd October 15, 2026 12:24

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up at 0b73128. Thanks for the quick fixes: the Dockerfile now copies the CSV, and the pt-BR hint says "carregue". Both check out.

Two more changes before merge.

1. Give the empty state room, sized to the window. Today the figure sits 20px under the column headers at every window size, and all the spare space collects below it: 117px at 1280×720, 837px at 2560×1440. The change below centers the empty state in the rest of the panel and scales the figure with the window. The figure is 202px wide at 1280×720, 302px at 1920×1080 and 360px at 2560×1440, and never narrower than 160px. I rendered it in the real layout from 1280×720 to 2560×1440, with no page scroll at any size.

  • MapProjects.jsx: one-click suggestion inline.
  • NoResults.jsx is outside the diff, so its change is below. graphicWidth is optional, so the other two callers (Candidates.jsx, SearchResults.jsx) keep the fixed 236×139 figure.
-const NoResults = ({searchedText, height, text}) => {
+const NoResults = ({searchedText, height, text, graphicWidth}) => {
@@
-      <SvgIcon style={{width: "236px", height: "139px", fill: "none"}} viewBox="0 0 236 139">
+      <SvgIcon style={{width: graphicWidth || "236px", height: graphicWidth ? "auto" : "139px", aspectRatio: "236 / 139", fill: "none"}} viewBox="0 0 236 139">

2. Download the sample from ocl-docs, so it can change without a Mapper release. The sample is now also in OpenConceptLab/ocl-docs#71, at docs/source/oclmap/samples/mapper_sample_lab_tests.csv, with the same content. A plain link to raw.githubusercontent.com doesn't download: browsers ignore download on a link to another origin, and GitHub serves the file as text/plain, so it opens as text in the tab. Fetching it and saving it as a blob does work, because GitHub sends Access-Control-Allow-Origin: *.

Keep the bundled copy and its Dockerfile line as the fallback, for networks that block raw.githubusercontent.com. Until ocl-docs#71 merges, the fallback serves the file, so merge order doesn't matter.

At module level, next to NEW_PROJECT_BLOCK_ERROR_CODES (outside the diff):

// The sample lives in ocl-docs so it can change without a Mapper release
// (OpenConceptLab/ocl_issues#2903). The bundled copy covers networks that
// block raw.githubusercontent.com.
const SAMPLE_SPREADSHEET_URL = 'https://raw.githubusercontent.com/OpenConceptLab/ocl-docs/main/docs/source/oclmap/samples/mapper_sample_lab_tests.csv'
const SAMPLE_SPREADSHEET_FALLBACK_URL = '/mapper_sample_lab_tests.csv'

const getCSV = url => fetch(url).then(response => response.ok ? response.blob() : Promise.reject(response.status))

const onSampleSpreadsheetClick = event => {
  event.preventDefault()
  getCSV(SAMPLE_SPREADSHEET_URL).catch(() => getCSV(SAMPLE_SPREADSHEET_FALLBACK_URL)).then(blob => {
    const url = URL.createObjectURL(blob)
    const link = Object.assign(document.createElement('a'), {href: url, download: 'mapper_sample_lab_tests.csv'})
    document.body.appendChild(link)
    link.click()
    link.remove()
    setTimeout(() => URL.revokeObjectURL(url), 1000)
  })
}

And the link (line 222):

<Link href={SAMPLE_SPREADSHEET_FALLBACK_URL} download onClick={onSampleSpreadsheetClick} sx={{fontSize: '14px'}}>

I tested it in the Mapper itself (Chromium, stubbed API), with all three changes applied. Before ocl-docs#71 merges, the GitHub request returns 404 and the bundled copy downloads, all 10 rows. With the file served from GitHub, the GitHub copy downloads. Either way the file is saved as mapper_sample_lab_tests.csv and the page doesn't navigate. CI's lint command exits 0, and npm run test:all passes. Safari isn't tested. The one-second delay before releasing the blob URL is the usual precaution for it.

Note, not for this PR: at phone width, the table's 650px minimum width pushes this empty state off-screen, and the header's "New Mapping Project" button too. That's the same on main.

Comment thread src/components/map-projects/MapProjects.jsx Outdated
@paynejd

paynejd commented Oct 15, 2026

Copy link
Copy Markdown
Member

To be clear- you are approved to merge after you make the above changes.

@snyaggarwal
snyaggarwal merged commit f96fdbf into main Oct 16, 2026
1 of 2 checks passed
@snyaggarwal
snyaggarwal deleted the issues#2903 branch October 16, 2026 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mapper: a new user's empty project list explains the first step and offers a sample spreadsheet

2 participants