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

WIP: (feat) - debugging tables #1512

Closed
wants to merge 4 commits into from
Closed

Conversation

JoviDeCroock
Copy link
Member

This is a first try, I don't know all use cases for tables so I would really appreciate a second/... pair of eyes on this.

Thanks in advance!

@coveralls
Copy link

coveralls commented Apr 4, 2019

Coverage Status

Coverage decreased (-70.9%) to 29.067% when pulling ab3d9e7 on feat/debugHtmlMarkup into 98f0d7e on master.

@developit
Copy link
Member

I'm thinking this might not work if there are components in the tree, right?

const TableRow = ({ a, b, c }) => <tr><td>{a}</td><td>{b}</td><td>{c}</td></tr>
render(<table>{data.map(row => <TableRow {...row} />)}</table>, document.body)

Just a thought as I read through: what about triggering validation when encountering a child that can only be nested in a given parent (rather than when encountering a parent that can only accept certain children)? Something like:

function getParentType(vnode) {
  return vnode._dom && vnode._dom.parentNode && vnode._dom.parentNode.localName;
}
options.diffed = vnode => {
  if (vnode.type == 'tr') {
    const parentType = getParentType(vnode);
    if (parentType !== 'tbody' && parentType !== 'thead') {
      console.warn(`Warning: improper nesting of table row\n<tr> can only be placed inside <tbody> or <thead>.\n    ${serializeVnode(vnode)}`);
    }
  }
};

@JoviDeCroock
Copy link
Member Author

JoviDeCroock commented Apr 5, 2019

Good point, I'll rewrite it for this case and add tests for it aswell. parentNode never seems to be defined though :/

@JoviDeCroock JoviDeCroock changed the title (feat) - debugging tables WIP: (feat) - debugging tables Apr 5, 2019
@marvinhagemeister marvinhagemeister self-assigned this Apr 7, 2019
@andrewiggins
Copy link
Member

FWIW, now that we have the _parent pointer, checking that the first parent DOM vnode is of type tr, etc. shouldn't be too hard

@JoviDeCroock
Copy link
Member Author

@andrewiggins I'll try to pick this up asap

@JoviDeCroock JoviDeCroock deleted the feat/debugHtmlMarkup branch October 10, 2019 23:11
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.

None yet

5 participants