add blockquotes support #72
Conversation
adds a mutationObserver that dispatches an event when roam is loaded. also adds event handlers and intervals called roam-toolkit-tick which allow us to affect rendering of content already on the page.
| import '../../features/fuzzy_date' | ||
|
|
||
| //detect when roam is loaded |
There was a problem hiding this comment.
I'm not certain that this is the best place for this code, but it works. Let me know if there is a better place. A lot of other features I will submit PR's for will use this functionality.
There was a problem hiding this comment.
Yeah, I'd put it somewhere in srs/ts/rendering and then import it here
There was a problem hiding this comment.
I dont see a folder or file named rendering, do you intent for me to create it?
| name: 'Blockquote Support', | ||
| settings: [{type: 'string', id: 'blockquote_prefix', initValue: '> ', label: 'Blockquote Prefix'}], | ||
| description: | ||
| 'If you start a block with these characters, custom styles will be added to show the block as a blockquote.', |
There was a problem hiding this comment.
I don't believe this description is actually being displayed anywhere - it would be good to get it shown in the menu or at least (or in addition to) when you select the item and are viewing the settings for it. I tried to make that happen but I wasn't sure of it and didn't want to tackle it as part of this PR.
There was a problem hiding this comment.
Ah indeed. Showing it in the expanded view for the setting and maybe on hover of the setting sounds good, bu you're right this is a matter for a different PR
| import '../../features/fuzzy_date' | ||
|
|
||
| //detect when roam is loaded |
There was a problem hiding this comment.
Yeah, I'd put it somewhere in srs/ts/rendering and then import it here
| import '../../features/fuzzy_date' | ||
|
|
||
| //detect when roam is loaded |
There was a problem hiding this comment.
make this into function with the corresponding name instead of grouping the code with comment
| characterData: true, | ||
| }) | ||
| //console.log('Observer running...') |
| name: 'Blockquote Support', | ||
| settings: [{type: 'string', id: 'blockquote_prefix', initValue: '> ', label: 'Blockquote Prefix'}], | ||
| description: | ||
| 'If you start a block with these characters, custom styles will be added to show the block as a blockquote.', |
There was a problem hiding this comment.
Ah indeed. Showing it in the expanded view for the setting and maybe on hover of the setting sounds good, bu you're right this is a matter for a different PR
| const observer = new MutationObserver(function (mutations) { | ||
| mutations.forEach(function (mutation) { | ||
| const newNodes = mutation.addedNodes // DOM NodeList |
There was a problem hiding this comment.
um just use type instead of a comment?
There was a problem hiding this comment.
Before doing all of the work to convert it to typescript I wanted to get this in to get feedback on if this was going to work for you at all. I will clean up things like this before submitting the final PR.
| //after that, run the scripts every 15 seconds indefinately | ||
| setInterval(function () { | ||
| document.dispatchEvent(new CustomEvent('roam-toolkit-tick', {})) |
There was a problem hiding this comment.
I'm not entirely sure why do we need to do this actually? If I understand MutationObserver correctly - we can observe changes in the individual blocks instead. Which seems like it'd provide a more responsive results vs periodically iterating over all the blocks?
I've done a similar thing with Asana tasks a while ago: https://github.com/Stvad/Asana-counter/blob/master/Asana%20counter.user.js#L26
There we observe each existing task, and have a separate thing to enable observation on the new blocks as they are loaded. Not sure if that's the best approach (maybe we can just have observer for any kind of changes in the blocks?) but seems like doing something similar should allow us to do a more targeted rendering..
There was a problem hiding this comment.
Because I don't believe that we can use an observer to notice changes to all current and future changes, and because you need it to run these functions even if no changes are being made - the contents can change from other users or when you navigate to a new page but have not yet made changes.
Without being in control of the underlying code, and especially since we expects this code to change, this is by far the safest option.
| return true | ||
| } | ||
|
|
||
| const applyCSS = function () { |
There was a problem hiding this comment.
reuse function from custom-css feature (probably adding optional id parameter). probably good idea to extract it into a separate utils file too
| } | ||
|
|
||
| async function blockquotes() { | ||
| const blocks = document.querySelectorAll('div.roam-block') |
There was a problem hiding this comment.
I like how https://github.com/roam-unofficial/roam-toolkit/pull/63/files#diff-e8280a2ce440ae79385e21c80dab2401 defines dedicated list of selectors, I think it'd be a good idea to steal that for selectors you use :)
|
|
||
| async function blockquotes() { | ||
| const blocks = document.querySelectorAll('div.roam-block') | ||
| for (const block of blocks as NodeListOf<Element>) { |
There was a problem hiding this comment.
this is more relevant if we stay with the time-based rendering and when we add more renderings but: it probably makes sense to have a function that will get all the blocks and then apply all transformations we have to each of them vs having each transformation fetch the blocks.
| applyCSS() | ||
| blockquotes() | ||
| }) | ||
| document.addEventListener('roam-toolkit-tick', function () { |
There was a problem hiding this comment.
as a general style thing - I'd prefer to use arrow functions (applies for all similar cases)
adds a mutationObserver that dispatches an event when roam is loaded.
also adds event handlers and intervals called roam-toolkit-tick which allow us to affect rendering of content already on the page.