MoOx / phenomic

DEPRECATED. Please use Next.js instead.
https://nextjs.org
MIT License
3.21k stars 248 forks source link

[Markdown] Headings rendering with # text #1105

Closed bherila closed 7 years ago

bherila commented 7 years ago

Is anyone else seeing that when you define headings in your .md files like # Heading 1 text or ## Heading 2 text, that there is a "#" character inserted before the heading displays?

<h2 id="contact-us">
<a href="#contact-us" class="phenomic-HeadingAnchor">#</a>
<!-- react-text: 135 -->Contact Us<!-- /react-text -->
</h2>

I am going to fix this for now by adding .phenomic-HeadingAnchor{ display: none} to my css file. I propose that the default website should not display a "#" character next to headings as this is unexpected. I actually thought for a moment that this was a bug!!

Happy to send a PR if others agree. Let me know 😄

thangngoc89 commented 7 years ago

This is actually a feature. But as this keep comes up, I suggest we disable this by default.
@bherila If you want to send a PR, remove autoLinkHeading in this file https://github.com/phenomic/phenomic/blob/fa314107debb6ca1f860b7c7a38dc30b0e2c8b6b/packages/plugin-transform-markdown/src/remark-plugins.js#L18-L27

bherila commented 7 years ago

Oh, I didn't convey my message clearly. I agree that the autoLinkHeading is a worthy feature. However, my proposition is that the "#" element is hidden by default. This way, hash links can still work, but they don't generate unexpected visible elements on the page.

DavidWells commented 7 years ago

I agree. I had to hide this element as well.

Any visable features like this should be opt in (in my opinion)

The auto anchor tag is great :)

One note on the auto anchor tag, it breaks of you use any special characters ( /, (, ), etc) in the heading text. Probably need to encode or strip those out from the hash

MoOx commented 7 years ago

Will remove it then :)

Feel free to open an issue if there is a bug. For slugs it's probably a remark-{something} issue :D

hellais commented 7 years ago

Are there plans to also add richer customisation of how the BodyRender works? What would be great is if it was possible to expose the remarkReactComponents to the caller so I can say:

const customComponents = {
  a: Link,
  p: Text
}
const page = () => ( 
<BodyRenderer remarkReactComponents={customComponents} />
)

This is a basic requirement for being able to make the most of using styled-components as usually you don't define global styles for classes, but rather rely on a styling the individual components.

Edit: I noticed this comment in the code, I don't know enough about the architecture of phenomic to understand why JSON is part of the equation here, but I suspect it's not so trivial to do what I am requesting.

MoOx commented 7 years ago

@hellais actually it's already possible via the components props. See https://github.com/phenomic/phenomic/blob/137ca8c96b0033ac80300900e03cda7d65fb27a1/packages/plugin-renderer-react/src/components/BodyRenderer.js#L13 Tell me if that's enough for you. Will try to document this.

hellais commented 7 years ago

Tell me if that's enough for you. Will try to document this

Ah, yes that is actually exactly what I was looking for. Sorry I missed that part. Maybe adding a note to the docs would also be useful. Thanks 👍

MoOx commented 7 years ago

Note that we already have a better Link that what rr 3 is exposing, so be sure to not blindly override it ;)

MoOx commented 7 years ago

This # has been removed by default since a few releases.