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

Invalid DOM property colspan. Did you mean colSpan #302

Closed
lemcii opened this issue Dec 20, 2022 · 2 comments · Fixed by #399
Closed

Invalid DOM property colspan. Did you mean colSpan #302

lemcii opened this issue Dec 20, 2022 · 2 comments · Fixed by #399
Labels
bug Something isn't working

Comments

@lemcii
Copy link

lemcii commented Dec 20, 2022

What happened?

When using {% colspan=2 %} or {% rowspan=2 %} this warning or equivalent is logged serverside.

To reproduce

{% table %}

- 1
- 2
- 3

---

- 1
- 2 & 3 {% colspan=2 %}

---

- 1
- 2 & 3 {% colspan=2 %}

{% /table %}

Version

0.2.1

Additional context

@markdoc/next.js

@lemcii lemcii added the bug Something isn't working label Dec 20, 2022
@adreyfus-stripe
Copy link

@lemcii Thanks for filing, your bug report makes sense. React expects DOM attributes to be camelCase rather than lowercase so that warning is coming from React. Let me circle back with the team and see how we want to fix this in our react renderer meanwhile should be safe to ignore for now since React will safely pass colspan through to the DOM: https://reactjs.org/blog/2017/09/08/dom-attributes-in-react-16.html

@rpaul-stripe
Copy link
Contributor

These attributes are defined in Markdoc's built-in schema for the td node type. It's possible to override what the render tree outputs for those attributes by adding a render property to the schema definition:

attributes: {
    colspan: { type: Number, render: 'colSpan' },
    rowspan: { type: Number, render: 'rowSpan' }
}

It's a bit of a tricky situation because we really only want them to have this camel casing for the React renderer, it's not applicable to the HTML renderer. There are a few different ways we could approach it, but maybe what we want to do is apply the casing in the built-in schema definition and then have the HTML renderer just call toLowerCase on its attribute values so that the HTML output is still conforming with standards. I will collect some feedback from the team and then move forward with that change if there's consensus.

mfix-stripe added a commit that referenced this issue Jun 2, 2023
mfix-stripe added a commit that referenced this issue Jun 2, 2023
* Use colSpan and rowSpan

Closes #302

* add marktest test too
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants