feat: introduce wrapperDiv option
#61
Conversation
|
|
wrapperDiv option
src/types.ts
Outdated
| @@ -177,7 +184,7 @@ export interface Options { | |||
| wrapperClasses?: string | string[] | undefined | null | ((id: string, code: string) => string | string[] | undefined | null) | |||
|
|
|||
| /** | |||
| * Component name to wrapper with | |||
| * Component name to wrap with (will wrap the wrapperDiv if present) | |||
There was a problem hiding this comment.
I wanted to clarify the relationship between wrapperDiv and wrapperComponent. i.e. explain that if wrapperDiv is set to true and a wrapperComponent is defined, the output will look like this:
<WrapperComponent>
<div class='markdown-body'>
[markdown here]
</div>
</WrapperComponent>We could maybe expand the text in the brackets to 'this will wrap the wrapper div when wrapperDiv=true'
Alternatively, we could remove if it is not worth mentioning (and assume that it is fairly easy to work out what combination of options will generate the desired behaviour).
There was a problem hiding this comment.
I see, if you mean on the wrapping order, I think it's fine to keep as-is, as it doesn't make much sense to wrap <div outside of the components anyway.
There was a problem hiding this comment.
Thanks for merging :)
This PR adds a boolean option
wrapperDivthat makes the wrapper div optional in a way that doesn't break the existing API.I can see the value of the wrapper div in some situations but don't see a reason for it to be required.
And when a
wrapperComponentis specified the extra div feels especially unnecessary and gets in the way.Making it optional allows for cleaner simpler markup and makes, for example, styling easier.
Apologies - I made the PR before discussing but let me know what you think / if it is mergable (or not)
Also, just to say, thanks for a great plugin!!