Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fix #3038 issue loading Grapher tables #3039

Closed
wants to merge 1 commit into from

Conversation

larsyencken
Copy link
Contributor

The issue is that loading a Grapher chart with tab=table in the URL fails, though navigating to the table does not.

It appears to be due to a typo in the guard when creating a table caption, fixed in this change.

The issue is that loading a Grapher chart with `tab=table` in the URL
fails, though navigating to the table does not.

It appears to be due to a typo in the guard when creating a table caption,
fixed in this change.
@larsyencken
Copy link
Contributor Author

The fix seems to work, but getting some DataTable unit test failures now:

FAIL jsdom packages/@ourworldindata/grapher/dist/tests/dataTable/DataTable.jsdom.test.js
  ● when you render a table › renders a variable name in the caption

    Method “text” is meant to be run on 1 node. 0 found instead.

      33 |     it("renders a variable name in the caption", () => {
      34 |         const cell = view.find(".DataTable .caption")
    > 35 |         expect(cell.text()).toContain("Child mortality")
         |                     ^
      36 |     })
      37 |
      38 |     it("renders a unit name in the caption", () => {

      at ReactWrapper.single (node_modules/enzyme/src/ReactWrapper.js:1168:13)
      at ReactWrapper.single [as text] (node_modules/enzyme/src/ReactWrapper.js:[62](https://github.com/owid/owid-grapher/actions/runs/7299432810/job/19892379882#step:7:63)9:17)
      at Object.text (packages/@ourworldindata/grapher/src/dataTable/DataTable.jsdom.test.tsx:35:21)

  ● when you render a table › renders a unit name in the caption

    Method “text” is meant to be run on 1 node. 0 found instead.

      38 |     it("renders a unit name in the caption", () => {
      39 |         const cell = view.find(".DataTable .caption .unit")
    > 40 |         expect(cell.text()).toContain("percent")
         |                     ^
      41 |     })
      42 |
      43 |     it("renders 'percent' in the caption when unit is '%'", () => {

      at ReactWrapper.single (node_modules/enzyme/src/ReactWrapper.js:1168:13)
      at ReactWrapper.single [as text] (node_modules/enzyme/src/ReactWrapper.js:629:17)
      at Object.text (packages/@ourworldindata/grapher/src/dataTable/DataTable.jsdom.test.tsx:40:21)

  ● when you render a table › renders 'percent' in the caption when unit is '%'

    Method “text” is meant to be run on 1 node. 0 found instead.

      43 |     it("renders 'percent' in the caption when unit is '%'", () => {
      44 |         const cell = view.find(".DataTable .caption .unit")
    > 45 |         expect(cell.text()).toContain("percent")
         |                     ^
      46 |     })
      47 |
      48 |     it("renders a row for each country", () => {

      at ReactWrapper.single (node_modules/enzyme/src/ReactWrapper.js:11[68](https://github.com/owid/owid-grapher/actions/runs/7299432810/job/19892379882#step:7:69):13)
      at ReactWrapper.single [as text] (node_modules/enzyme/src/ReactWrapper.js:629:17)
      at Object.text (packages/@ourworldindata/grapher/src/dataTable/DataTable.jsdom.test.tsx:45:21)

@sophiamersmann
Copy link
Member

Hey @larsyencken, I'm just seeing this now. My best guess is that this happens because Grapher attempts a render although it hasn't finished loading the data. Let me quickly try a fix!

@sophiamersmann
Copy link
Member

That was it! I opened a PR here: #3042

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.

2 participants