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

"rowspan" attribute does not appear in DOM #264

Closed
ericclemmons opened this issue Aug 14, 2013 · 9 comments
Closed

"rowspan" attribute does not appear in DOM #264

ericclemmons opened this issue Aug 14, 2013 · 9 comments

Comments

@ericclemmons
Copy link
Contributor

<th rowspan="2"></th> does not set rowspan in the DOM.

Component:

render: function() {
  return (
    <thead>
      <tr>
        {this.props.columns.map(this.renderColumnLabel)}
        <th rowspan="2"></th>
      </tr>
      <tr>
        {this.props.columns.map(this.renderColumnControl)}
      </tr>
    </thead>
  );
}

Output:

...
<th data-reactid=".r[5je1f].[0].[1].[1].[6].[0].[0].[1].[0].[0].[0].[1]"></th>
...

Can this be my first PR? :)

@sophiebits
Copy link
Collaborator

If you do rowSpan it'll work -- we use the camelcase version for attributes for consistency with the DOM interface where you'd do el.rowSpan = 2;. I've already opened #255 to warn when using the wrong case since lots of people get confused by this.

@ericclemmons
Copy link
Contributor Author

Oh, that's it. Thanks :) I'll await #255's resolution for long term, but adjust my code in the short term...

@zpao
Copy link
Member

zpao commented Aug 14, 2013

Sorry @ericclemmons, it could have been :) I tried to do a pass for missing attributes pre-0.4 but if you notice anything missing from https://github.com/facebook/react/blob/master/src/dom/DefaultDOMPropertyConfig.js, feel free to open an issue or PR

@ericclemmons
Copy link
Contributor Author

Oh, it totally looks like rowSpan (camelCased!) is missing. I'll submit a PR...

@ericclemmons ericclemmons reopened this Aug 14, 2013
@sophiebits
Copy link
Collaborator

Oops! I saw colSpan and naively assumed that rowSpan would be there too… :)

@zpao
Copy link
Member

zpao commented Aug 23, 2013

@ericclemmons Still interested in fixing this?

@ericclemmons
Copy link
Contributor Author

Closing for #291...

@inimino
Copy link

inimino commented May 4, 2014

Given that this trips people up, instead of being consistent with the DOM, why not be consistent with HTML and support all-lowercase attributes?

I just had to go back to the docs for this issue about the "autocomplete" attribute.

I'm writing something that looks like HTML, it should work like HTML... otherwise it's an inconsistency, not consistency.

@ericclemmons
Copy link
Contributor Author

I agree. It would be preferable (to me) if attributes were all mapped .toLowerCase().

bvaughn pushed a commit to bvaughn/react that referenced this issue Aug 13, 2019
Fetch owners list from renderer (using suspense)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants