Automatically re-rendering when inline markdown changes about zero-md HOT 8 CLOSED

Hi @EmilePerron, thanks so much for reaching out - I think it's a great feature and I'd love a PR! 😁

On first pass, this may not be a trivial feature to implement. Some thoughts come to mind:

1. Probably both <script> and style <template> tags should be observed for changes.

2. I guess mutation callbacks must be debounced so multiple calls will only trigger a re-render once. (I didn't read in detail on MutationObserver specs though, maybe it's native?)

3. Perhaps the render() function can be re-factored to support conditional DOM stamping (or re-building)? Something like if render() is called twice in a row, checks are added so that the second call shouldn't actually re-stamp i.e. render() will only render changes.

There are probably other optimisations and gotchas too, so let's keep this thread open for discussion! Also do let me know if there's anything I can help with. Thanks so much for you time once again. 👍

from zero-md.

@zerodevx Glad to see I'm not the only one interested in this!

Here are some of my thoughts after reading your comment and doing some exploration of the code:

1. Agreed - both styles and content could be observed and trigger partial re-renders.
2. The MutationObserver will only trigger once in situations where multiple changes occur one after another, unless there are timeouts, delays or painting in between changes. So I don't think debouncing will be necessary. (as other changes often take place around the component at the same time, even a 1ms debounce makes the component seem slightly sluggish compared to non-debounced content and elements around it).
3. Perhaps render() could be refactored to handle both the full rendering and the "partial" rendering, but that would mean the method has multiple purposes, and would do different things in subsequent calls. This might be more confusing to users and developers alike. It could also break existing codebases that manually call render(). Making new methods to support the partial rendering/replacement of styles and content seems wiser.

Those are my early thoughts - once again, feel free to chime in if you disagree or have anything else to add!

I'll keep working on a PR in my free time; a PR will come as soon as I've got something tangible to share 😉

from zero-md.

All sounds good so far! Looking forward to this - thanks for taking the time! 🎉

from zero-md.

Just opened a WIP PR for this here: #51 . Here's the gist of it:

• Insertion and removal of <template> and <script type="text/markdown"> tags in the component are detected with a MutationObserver and trigger a partial re-rendering of styles and/or content (via the new refreshStyle() and/or refreshContent())
• Updates to <template> and <script type="text/markdown"> elements in the component also trigger partial re-rendering of the appropriate part of the component, using the same methods.
• When a partial re-rendering occurs, the zero-md-rendered event is fired just like in the render() method. However, it includes a partial flag and a part indicator in the event.detail (ex.: this.fire('zero-md-rendered', { partial: true, part: 'body' }))
• This feature is inactive when the component is in manual-render mode.

I have included simple test cases for both styles and content changes, which are both passing at the moment.

More tests could likely be added to ensure there's no regression in relation with other features (ex.: no-shadow, manual-render, more complex manipulations, etc.).

You'll likely want to review the code style - I tried to match yours as closely as I could (and lint checks all pass), but I tend to be very verbose and space heavy, so it might not be up to your standards 😬

Let me know if you have any feedback.
Cheers!

from zero-md.

Wow that's super quick! 👍 Really appreciate your contribution! Do give me some time to review/test in detail - I'll update you as soon as I can. Thanks once again!

from zero-md.

Hello @zerodevx 👋

Have you had some time to look into the #51?
If not, would it be possible to get a [very approximative] ETA?

I have a workaround that works just fine for development purposes in my project, so there's no real rush - just curious 🤓

Cheers!

from zero-md.

Hi @EmilePerron thanks for your note - my apologies! I'll try to clear my open-source debts this weekend! 🙏

from zero-md.

Hi @EmilePerron

My apologies for the wait. I (finally) reviewed PR #51 and it generally looks very good.

I have included simple test cases for both styles and content changes, which are both passing at the moment.

Thanks for adding the tests. They look very good!

Insertion and removal of <template> and <script type="text/markdown"> tags in the component are detected with a MutationObserver and trigger a partial re-rendering of styles and/or content (via the new refreshStyle() and/or refreshContent())

Updates to <template> and <script type="text/markdown"> elements in the component also trigger partial re-rendering of the appropriate part of the component, using the same methods.

I still feel there's an opportunity to refactor the render() method to render changes instead, which removes the need for refreshStyle() and refreshContent() - and IMO simplifies the codebase as well.

When a partial re-rendering occurs, the zero-md-rendered event is fired just like in the render() method. However, it includes a partial flag and a part indicator in the event.detail (ex.: this.fire('zero-md-rendered', { partial: true, part: 'body' }))

Thanks for the recommendation. I'll document this API change.

This feature is inactive when the component is in manual-render mode.

Good catch!

More tests could likely be added to ensure there's no regression in relation with other features (ex.: no-shadow, manual-render, more complex manipulations, etc.).

Coverage seems OK so far; we can add as and when bug reports surface. 😄

You'll likely want to review the code style - I tried to match yours as closely as I could (and lint checks all pass), but I tend to be very verbose and space heavy, so it might not be up to your standards 😬

Thanks for doing this! I don't personally have big opinions on this - as long as it passes standardjs linting it's an OK.

I'll probably merge this upstream in a bit - I'll also see if there're opportunities to simplify, if not today then soon. Thanks so much!

from zero-md.