phrasing content within a button #368
Conversation
|
/cc @seancurtis |
|
I have run over these changes with @seancurtis and he is happy with them |
| // function to keep things simple. | ||
| // There is no harm checking if the parent is has an interactive tag name even if it cannot have | ||
| // any children. We need to perform this loop anyway to check for the contenteditable attribute | ||
| const hasAnInteractiveTag: boolean = Boolean(interactiveTagNames[current.tagName.toLowerCase()]); |
There was a problem hiding this comment.
Any reason why you're using the Boolean constructer to coerce to boolean over !!? !! is more performant but if this isn't being called a lot then it won't make a difference here. Micro optimisations aren't worth it :) https://jsperf.com/bool-vs-doublenot
There was a problem hiding this comment.
There was a problem hiding this comment.
It also returns a value (it would return an Boolean object if we used new Boolean(value)
There was a problem hiding this comment.
Rather that using a double negative to cast - we are super explicitly casting to boolean :)
| // See 'Permitted content' on https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button | ||
| // Rather than having two different functions we can consolidate our checks into this single | ||
| // function to keep things simple. | ||
| // There is no harm checking if the parent is has an interactive tag name even if it cannot have |
| phrasing content | ||
| </a> | ||
| </p> | ||
| <button>hello <strong>world</strong></button> |
There was a problem hiding this comment.
Should we check the case where there are multiple levels of nesting?
There was a problem hiding this comment.
This is not a test file - it is simply a storybook. I have added tests for a parent, but not multi levels of parentage. I am not sure if a test would be super valueable - thoughts?
|
|
||
| it('should start a drag if the target is not an interactive element', () => { | ||
| const nonInteractiveTagNames: TagNameMap = { | ||
| a: true, |
There was a problem hiding this comment.
Just out of interest, is there any reason why anchors are not classed as interactive elements?
There was a problem hiding this comment.
Their only interaction is a click. We actually allow draggables and drag handles to be anchors - so they can be both clickable as well as draggable. We manage this interaction separately. We completely support an anchor being an anchor as well as draggable. This feels right too. The other elements are more form elements and do not make sense as a thing if you cannot interact with it directly.
Does that help?
Fixes #359