Add "id" to headings #709
Add "id" to headings #709
Conversation
| : React.Children.toArray(child.props.children).reduce(flatten, text); | ||
| } | ||
|
|
||
| function HeadingRenderer(props: any) { |
There was a problem hiding this comment.
Hmm I'm not a huge fan of having a Component inline in another component, even though this is small one. @Angelmmiguel what do you think?
There was a problem hiding this comment.
Agree with @prydonius. If the component is specific to another one, I usually keep them in the same folder:
CommonComponent
- CommonComponent.test.tsx
- CommonComponent.tsx
- CommonComponent.css
- SpecificComponent
- SpecificComponent.test.tsx
- SpecificComponent.tsx
- SpecificComponent.css
| @@ -13,6 +13,23 @@ interface IChartReadmeProps { | |||
| version: string; | |||
| } | |||
|
|
|||
| // Code from https://github.com/rexxars/react-markdown/issues/69 | |||
| function flatten(text: string, child: any): any { | |||
There was a problem hiding this comment.
If we keep this function in this class, we should probably name it better, or instead just have it as an anonymous function in the .reduce
There was a problem hiding this comment.
I moved it to a different component file. I prefer to leave it as it is to maintain fidelity to the linked issue.
| : React.Children.toArray(child.props.children).reduce(flatten, text); | ||
| } | ||
|
|
||
| function HeadingRenderer(props: any) { |
There was a problem hiding this comment.
Agree with @prydonius. If the component is specific to another one, I usually keep them in the same folder:
CommonComponent
- CommonComponent.test.tsx
- CommonComponent.tsx
- CommonComponent.css
- SpecificComponent
- SpecificComponent.test.tsx
- SpecificComponent.tsx
- SpecificComponent.css
| : React.Children.toArray(child.props.children).reduce(flatten, text); | ||
| } | ||
|
|
||
| const HeadingRenderer: React.SFC<{}> = (props: any) => { |
There was a problem hiding this comment.
Shouldn't we have written some tests for this btw?
There was a problem hiding this comment.
I didn't add a test for this because it's only meant to be used with the ReactMarkdown component so I added the test there.
Fixes #702
Add the "id" property to the headings when parsing markdown. The approach to do that is specified here:
remarkjs/react-markdown#69
The logic to transform the heading text into a link is the same than GitHub: https://github.com/jch/html-pipeline/blob/master/lib/html/pipeline/toc_filter.rb#L42