[Enhancement] re-enable pandoc div rendering#6121
[Enhancement] re-enable pandoc div rendering#6121benniekiss wants to merge 9 commits intoZettlr:developfrom
Conversation
benniekiss
changed the title
|
Column rendering almost works. However, It messes with the cursor positioning below the rendered columns, probably due to using 
|
benniekiss
force-pushed
the
enable-pandoc
branch
from
f2f74bc to
eeb7bb5
Compare
benniekiss
force-pushed
the
enable-pandoc
branch
2 times, most recently
from
41641c8 to
e1f90c1
Compare
benniekiss
force-pushed
the
enable-pandoc
branch
from
e1f90c1 to
579f12c
Compare
nathanlesage
left a comment
nathanlesage
left a comment
There was a problem hiding this comment.
In general this looks good, I only have a few concerns and questions.
| display: 'block !important', | ||
| // Prevent user-provided styling from altering the dom-layout | ||
| flex: 'initial !important', | ||
| height: 'initial !important', | ||
| width: 'initial !important', |
There was a problem hiding this comment.
Can't we get rid of the !important? We are already warning users that they need to be careful with Custom CSS, so I see no need to have that here. In addition, since the user CSS is always inserted last, it will always be able to override any styles we have anyways.
There was a problem hiding this comment.
This is to prevent css declared in the div attribute from altering the dom. Sometimes the attributes are auto-generated or are required for proper rendering in pandoc, but we dont want them taking effect in the editor renderer
| import type { SyntaxNode } from '@lezer/common' | ||
|
|
||
| function showDivSpanDecorations (view: EditorView): RangeSet<Decoration> { | ||
| function showSpanDecorations (view: EditorView): RangeSet<Decoration> { |
There was a problem hiding this comment.
Suggestion: This doesn't "show" inasmuch as it "creates" or "computes" these decorations, so I'd argue we should rename this function accordingly.
| return Decoration.set(ranges, true) | ||
| } | ||
|
|
||
| const overrideWrapper = BlockWrapper.create({ tagName: 'pandoc-div-info-wrapper' }) |
There was a problem hiding this comment.
Why would you call it overrideWrapper…?
There was a problem hiding this comment.
It overrides styling of the parent div, but I could try and come up with a better name
|
|
||
| const overrideWrapper = BlockWrapper.create({ tagName: 'pandoc-div-info-wrapper' }) | ||
|
|
||
| function showDivDecorations (view: EditorView): RangeSet<BlockWrapper> { |
There was a problem hiding this comment.
Same as with the "show" spans function above
| if (info) { | ||
| classes.unshift(view.state.sliceDoc(info.from, info.to)) | ||
| } |
There was a problem hiding this comment.
This looks a bit … dirty(?) to me? Are info strings contained within the attributes?
There was a problem hiding this comment.
no, info strings are parsed separately, same as code-info strings:
::: info {#id}
...
:::
The class needs to be inserted first to take priority
| } | ||
|
|
||
| const wrapper = BlockWrapper.create({ | ||
| tagName: 'pandoc-div-wrapper', |
There was a problem hiding this comment.
Any benefits to using a custom tag instead of a div, except increased semantic meaning? Are there any downsides we should keep in mind?
There was a problem hiding this comment.
I dont actually know if setting tagName: 'div' would create a regular div... I'll test, but my instinct is to keep it a separate element name to make it clear what is providing the wrapper
| ranges.push(wrapper.range(fromLine.to + 1, toLine.from - 1)) | ||
| ranges.push(overrideWrapper.range(fromLine.from, fromLine.to)) | ||
| ranges.push(overrideWrapper.range(toLine.from, toLine.to)) |
There was a problem hiding this comment.
Do I understand it correctly that you're creating two nested divs, one outside, and one inside? Do I understand it correctly that the overrideWrapper effectively just overrides styles based on the classes and attributes, whereas the wrapper itself styles the info string…? If so, we may want to think about the naming of these two to ensure we won't get confused in 6 months!
There was a problem hiding this comment.
I'll clarify this in the code, but this essentially creates three sibling divs:
::: {#id} <-- first (overrideWrapper)
...content... <-- second
::: <-- third (overrideWrapper)
Two reasons for this:
The first reason is to remove any styling from the mark nodes (:::) when they are nested. This makes it easier to read and find the boundaries of nested divs.
The second is that applying the styling over just the content makes it easier to see how the styling is rendered. This is most evident when using something like
::: {style="background-color: yellow; border-radius: 10px;"}
content
:::
The rounded corners will appear within the div marks. Otherwise, they would be rounded at the divmarks, and since background styling is removed from them, the rounded corners would not be visible.
* better UX -- since the mark lines hide styling, * things like border radius become hidden
* blockwrappers should not alter the dom layout, * so we disallow rendering styles, such as * `display: flex`, `width`, and `height`, which * would otherwise alter the layout
benniekiss
force-pushed
the
enable-pandoc
branch
from
ccc2916 to
d8b58c8
Compare
2 participants
Description
This PR re-enables pandoc div rendering by using the new
BlockWrapperapiChanges
The pandoc div and span renderers were separated because
BlockWrappershave a similar, but different, structure and typing thanDecorations.The div renderer was updated to use
BlockWrappers, which essentially wraps the given range in a separate HTML element. This provides better styling thanDecoration.linebecause the entire content is wrapped in one parent element.An additional blockwrapper was added around the opening and closing lines of the pandoc div to essentially revert the styling of parent divs. I did this so that those regions do not receive most styling from the parent pandoc div, making them easier to pick out when div rendering is enabled. This is a very simple override, and there is still some styling that applies. Another approach would be to use a widget across these regions, but that would have to be applied in a separate renderer function since those are applied as
Decorationobjects.Additional information
the
lightGreensection is a blockquote, which is why the font color is differentTested on: macOS 26/Fedora 43