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

Shortcode 'feature': fix output of invalid nested <p> tags #526

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

deining
Copy link
Collaborator

@deining deining commented Apr 19, 2021

This PR fixes invalid HTML output caused by by shortcode feature:

HTML source from docsy.dev:

<div class="col-lg-4 mb-5 mb-lg-0 text-center ">
  <div class="mb-4 h1">
    <i class="fas fa-lightbulb"></i>
  </div>
  <h4 class="h3">See Docsy in action!</h4>
  <p class="mb-0"><p>As well as our example site, there’s a growing number of projects using Docsy for their doc sites.</p>
</p>
  <p><a href="/docs/examples/">Read more …</a></p>
</div>

Note that markup of paragraph As well as our example site, there’s ... contains nested <p> tags, which is invalid.

@deining
Copy link
Collaborator Author

deining commented Apr 20, 2021

I just realized that shortcode lead treats its .Inner content either as markdown or as HTML, depending on the extension of the page that makes use of the the shortcode:

{{ if eq .Page.File.Ext "md" }}
    {{ .Inner | markdownify }}
{{ else }}
    {{ .Inner | htmlUnescape | safeHTML }}
{{ end }}

Does it make sense to implement this for shortcode feature in a similar way?

@LisaFC
Copy link
Collaborator

LisaFC commented Apr 27, 2021

I just realized that shortcode lead treats its .Inner content either as markdown or as HTML, depending on the extension of the page that makes use of the the shortcode:

{{ if eq .Page.File.Ext "md" }}
    {{ .Inner | markdownify }}
{{ else }}
    {{ .Inner | htmlUnescape | safeHTML }}
{{ end }}

Does it make sense to implement this for shortcode feature in a similar way?

It probably does - I think it's "fixed" in lead because it wasn't working on HTML pages, users might want to use feature in the same way.

@deining
Copy link
Collaborator Author

deining commented Apr 28, 2021

Does it make sense to implement this for shortcode feature in a similar way?

It probably does - I think it's "fixed" in lead because it wasn't working on HTML pages, users might want to use feature in the same way.

I rebased my PR and added the code passage that differentiates between .md and .html content. Hopfully my PR is ready for merging now.

@LisaFC
Copy link
Collaborator

LisaFC commented Apr 28, 2021

Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants