Repository navigation
Conversation
|
View your CI Pipeline Execution ↗ for commit 52a57d5 ☁️ Nx Cloud last updated this comment at |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3442 +/- ##
==========================================
- Coverage 91.34% 90.99% -0.36%
==========================================
Files 407 277 -130
Lines 6873 5950 -923
Branches 2210 1983 -227
==========================================
- Hits 6278 5414 -864
+ Misses 586 527 -59
Partials 9 9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
LinKCoding
left a comment
LinKCoding
left a comment
There was a problem hiding this comment.
Played around with the example locally and looks good to me :)
Left some non-blocking thoughts and maybe a reminder to clean up test code?
|
|
||
| Preview: Docs Embedding videos within a site creates lively and informative pages that are attractive to visitors. Like the `<img>` element, the `<video>` element requires a src attribute with a link to the video source. Unlike the `<img>` element however, the `<video>` element requires an opening and a closing tag. | ||
|
|
||
| <video> |
There was a problem hiding this comment.
this lead me down a rabbit hole of when someone might write raw HTML and markdown does not like that lol
`<video>` test testtest ✅
Yoo yoo <pre><video> chck </video></pre> ❌
<p><code><video> chceeek </video></code></p> ❌
Seems like when using code fences (```) or inline backticks, the < and > characters get converted to < and > respectively, so there's no need for the override to kick in.
TL;DR: writing raw HTML in a markdown component is an edgecase, but probs not one worth worrying about
| "react-dom": "^19.0.0", | ||
| "react-helmet-async": "^2.0.5" | ||
| "react-helmet-async": "^2.0.5", | ||
| "react-player": "^3.4.0" |
There was a problem hiding this comment.
This is also test code, yeah? i.e. to make testing easier?
| ); | ||
|
|
||
| // a <p> that only holds video tags, i.e. a single-line <video></video> that marked parsed as paragraph text | ||
| export const isVideoParent = (node: HTMLToReactNode, tagName: string) => |
There was a problem hiding this comment.
nit: the name throws me off a little and the need to have tagName as a parameter
name: isVideoOnlyParagraph is a 🤖 suggestion.
parameter: doesn't hurt I guess, but given how specific the function name is, seems like it's always checking 'video' that it might not be needed
|
🚀 Styleguide deploy preview ready! Preview URL: https://6adf42dd5abfc17745f8fd81--gamut-preview.netlify.app |
|
📬 Published Alpha Packages:
|
Overview
Fixes Markdown rendering breaking on open video tags.
PR Checklist
Testing Instructions
PR Links and Envs