Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to render and edit FrontMatter #2751

Merged
merged 5 commits into from
Jul 29, 2022
Merged

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jul 24, 2022

Summary

This adds support to show and edit a FrontMatter block as a simple code block.

This does not introduce special parsing and rendering like suggested here: #145 (comment)
Because that requires to assume a language, while mostly YAML is used, I already saw a lot projects using TOML or even JSON in the FrontMatter. So using a simple code block allows us to not make any farfetched assumptions about the content while still gives the user a convenient way to edit.

I only added a hr element to visually separate FrontMatter from content.

You can even remove the FrontMatter if you like, and copy it, pasted FrontMatter will result in a simple code block as the FrontMatter is only allowed as the first element of a document.

What is not implemented: Adding a new FrontMatter to a document, while implementing this would be straightforward it would add more UI elements and so I would like to have some feedback if I should add such functionality / interface elements.


Example

Sourcecode

---
some: value
other: 2.3
---
# Heading
```
Code block
```

After / With this PR

Screenshot rendering with FrontMatter

Before / Without this PR

Screenshot rendering before

@susnux susnux added enhancement New feature or request 3. to review labels Jul 24, 2022
Copy link
Collaborator

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! I think it covers the main use case of being able to handle markdown files with front matter without adding much complexity. I'd personally stick to not exposing the front matter functionality in the ui - but if someone has a clever idea of how to handle this i'm all ears.

Two questions come to my mind:

  • What happens with front matter that is affected by the escape html? It would be nice if it would be preserved when opening the file in text. Maybe we could even add a test for this.
  • What happens if the front matter block is copied and pasted further down in the document? Does it add a frontmatter node?

The latter question makes me think it might be good to just define a new node type instead of inheriting code block. This would avoid inheriting the input rules, keyboard shortcuts and all that. The new node type could also not be part of the 'block' group and would then not be allowed in the document. Instead we could prefix the document content with a frontmatter? expression so front matter can only occur at the start of the document and only once.

@juliusknorr
Copy link
Member

Very nice :) I agree that we should probably think about how if and how we actually present this to the user.

  • Maybe we can collapse it by default and explicitly add a front matter label, however that is quite technical
    • On the other hand we would only expose this is someone already has frontmatter in their document
  • It could be separated visually a bit more to not look exactly like a code block, maybe a lighter background

Maybe @nimishavijay or @jancborchardt also have some input on this.

@jancborchardt
Copy link
Member

It seems reasonable to show it like a code block indeed. If Frontmatter is used, then it's likely understood and can be shown directly?

We can still do any improvements if issues come up, but don't need to overengineer at this stage. :)

@susnux
Copy link
Contributor Author

susnux commented Jul 25, 2022

Thank you for your feedback!

  • What happens with front matter that is affected by the escape html

Not much as it is reverted when serialized to markdown, it only sanitizes for the HTML output. (see below)

  • What happens if the front matter block is copied and pasted further down in the document?

The pasted one is transformed to a normal code block. (see below)

I'd personally stick to not exposing the front matter functionality in the ui - but if someone has a clever idea of how to handle this i'm all ears

I have some local changes which would introduce an input rule:
If you enter three dashes at the beginning of the document they would transform into a front matter node instead of a hr.
Does this solution sound reasonable to include?

It could be separated visually a bit more to not look exactly like a code block

Maybe this way? This are my local changes including slightly different styling and the input rule.

front-matter.mp4

@juliusknorr
Copy link
Member

I'd personally stick to not exposing the front matter functionality in the ui - but if someone has a clever idea of how to handle this i'm all ears

I have some local changes which would introduce an input rule: If you enter three dashes at the beginning of the document they would transform into a front matter node instead of a hr. Does this solution sound reasonable to include?

Yes, I think that is a nice feature to be able to add it even if we don't expose a button.

It could be separated visually a bit more to not look exactly like a code block

Maybe this way? This are my local changes including slightly different styling and the input rule.

Looks nice 👍

@susnux susnux force-pushed the feat/frontmatter branch 2 times, most recently from 849ec6f to 51df26d Compare July 25, 2022 19:29
@max-nextcloud max-nextcloud self-requested a review July 26, 2022 04:52
Copy link
Collaborator

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for overwriting the tiptap code block rules to make sure they don't apply later in the doc.

I think this is ready to go! 🚀

Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works like a charm, apart from the small comment about the "Front matter" label this is good to go from my side, so LGTM after that is addressed.

css/prosemirror.scss Outdated Show resolved Hide resolved
@susnux susnux force-pushed the feat/frontmatter branch from 51df26d to e0beb8a Compare July 26, 2022 08:56
@susnux
Copy link
Contributor Author

susnux commented Jul 26, 2022

Added some test, unfortunately cypress does not support copy paste (somewhat related to cypress-io/cypress#2386 ).
Also fixed serializing of a front matter containing a sequence of three or more dashes, this is valid but the fences must be longer than the sequence within the content.

@susnux susnux force-pushed the feat/frontmatter branch 4 times, most recently from 5d708c4 to 9e5082e Compare July 27, 2022 12:51
@susnux
Copy link
Contributor Author

susnux commented Jul 27, 2022

/compile

@susnux susnux requested a review from juliusknorr July 27, 2022 17:10
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome contribution @susnux Thanks a lot for that :)

@juliusknorr
Copy link
Member

Ah failure seems related to the recent server login page change ... waiting for #2756 then for the merge.

@susnux susnux force-pushed the feat/frontmatter branch from 09ac107 to 55992c9 Compare July 28, 2022 09:32
@susnux
Copy link
Contributor Author

susnux commented Jul 28, 2022

/compile

@juliusknorr juliusknorr mentioned this pull request Jul 28, 2022
20 tasks
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great now, nice work @susnux! :)

The blue line on the bottom we could instead do on the left like for the infoboxes – but that is not blocking and can be done in a follow-up.

@susnux
Copy link
Contributor Author

susnux commented Jul 28, 2022

Looks great now, nice work @susnux! :)

Thank you!

The blue line on the bottom we could instead do on the left like for the infoboxes – but that is not blocking and can be done in a follow-up.

I did this on purpose, to create some (visual) vertical separation (front matter vs file content).
But I am fine with changing it to match the infobox design, I have attached screenshots of both designs, so @jancborchardt if you like the second one more I will change it :)

Currently
current design

Proposed one
proposed design

@jancborchardt
Copy link
Member

jancborchardt commented Jul 28, 2022

Very cool @susnux! The proposed one (with just the blue bar on the left, no icon) is definitely the best. :) And it still looks consistent with the rest.

@susnux susnux force-pushed the feat/frontmatter branch from 6631441 to 24fb71c Compare July 28, 2022 14:11
@susnux
Copy link
Contributor Author

susnux commented Jul 28, 2022

/compile amend

@susnux
Copy link
Contributor Author

susnux commented Jul 28, 2022

The proposed one (with just the blue bar on the left, no icon) is definitely the best. :) And it still looks consistent with the rest.

Ok changed the design accordingly :)

susnux added 4 commits July 29, 2022 16:07
This adds support for parsing files containing FrontMatter and editing
it as like a code block.

Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
Added an input rule to add a front matter node if `---` is typed
in the very beginning of the document.
If it is typed somewhere else the default behavior is still
to insert a `hr` node.

Added a border on the left of the front matter, like the callouts
and blockquote, and added a title for the element.

Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>

a
Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
If the front matter contains a sequence of three or more dashes,
the front matter fences must be adjusted.

Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
@susnux susnux force-pushed the feat/frontmatter branch from d1db422 to 692494b Compare July 29, 2022 14:10
@susnux
Copy link
Contributor Author

susnux commented Jul 29, 2022

/compile amend

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Support Frontmatter in MD files
6 participants