-
Notifications
You must be signed in to change notification settings - Fork 22
fix: html semantic, block tag should be in block tag #442
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
Conversation
|
Preview is ready. |
| > | ||
| <Col sizes={titleSizes} className={b('content-inner')}> | ||
| {overtitle && ( | ||
| <p className={b('overtitle')}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why it's a div and not p?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the < p > tag in HTML cannot contain block-level elements, like < div >, according to the HTML5 content model. When an HTML parser encounters this scenario, it implicitly closes the < p > tag before the < div > starts and re-opens a new one after the < div > ends, resulting in the observed behavior.
In simple words: browsers automatically closed the p tag when they met div inside, cause they considered div not as a content of p, but rather as the next node after the p.
The description node looked like this:
<p class="pc-header-block__description">
<div class="yfm">
<!-- Typographed content started, this is not a part of any component -->
<p>Text...</p>
<!-- End of the content -->
</div>
</p>
In fact, browser renders it this way:
<p class="pc-header-block__description">
</p><div class="yfm">
<!-- Typographed content started, this is not a part of any component -->
<p>Text...</p>
<!-- End of the content -->
</div>
<p></p>
| <HTML block={hasBlockTag(title)}>{title}</HTML> | ||
| </h1> | ||
| {description && ( | ||
| <p className={b('description')}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why should we have a div here instead of p?
| {itemTitle && ( | ||
| <h5 className={b('item-title')}> | ||
| <HTML>{itemTitle}</HTML> | ||
| <HTML block={hasBlockTag(itemTitle)}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we relocate block check inside HTML-component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could do, but HTML component is used by YFMWrapper and YFMWrapper has prop to change flag 'block', so we have to extend type to 'undefined' for this flag to not override value from YFMWrapper in HTML component. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, check moved into HTML component
No description provided.