Fix details triggers that contain markup #1736
Conversation
Code quotes in headers are no longer auto-linked by `simpleLinker`, both for UX and bugfix purposes. Before, any markup like code quotes would break a header as only the first child had its text extracted. Now the trigger uses the whole children value of the header it's made from.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
it's fine to keep it disabled, looks like no one likes it |
There was a problem hiding this comment.
@rogermparent looks great, thanks! please take a look at the comments and address them as it makes sense.
This is unrelated to the PR, but while we're ts-linting we may as well
Now it just runs off the first (filtered) child. If that child is a header, it's assumed the last item of the heading is an auto-link and removed. Currently, every `details` first child is a heading, and all headings have auto-links.
This is mostly as an example of this PR's fix working.
Even if the element is a string, the regex test will return false from a non-string property.
|
All the feedback has been addressed, check out a live example of this working on the preview! The behavior now throws if the |
| @@ -7,34 +7,6 @@ | |||
| alone does. | |||
| */ | |||
|
|
|||
| async function createStaticGithubDataNode(api, fieldData = {}) { | |||
There was a problem hiding this comment.
Yeah, it gets picked up by lint-ts.
There was a problem hiding this comment.
hmm ... why didn't it pick it up earlier?
Before, as pointed out here by @jorgeorpinel, any markup like code quotes would break a header as only the first child
had its text extracted. Now, the
Collapsibleuses the wholechildrenvalue of theheader it's made from as the
triggertext, so anything that can go in a heading can go in adetailstrigger.simpleLinkeris also changed to not apply to code quotes within headings, partially because of this and also because auto-links in headers feels like it clutters the UX a bit since the same contents will very likely be linked in the following paragraph. I can re-enable this behavior if we find it desirable.