Skip to content

Commit

Permalink
Fix rendering markdown when updating open in new tab setting (#19356)
Browse files Browse the repository at this point in the history
* Fix rendering markdown when updating open in new tab setting (#18949)

- Moved set the link target behaviour outside the constructor
- Minor style fixes

* Refactoring: recreate markdownit if openLinksInNewTab changes. Add test

* Move props checking conditions to variables. Add test for whiteListedRules prop change

* Add checks before changing the props
  • Loading branch information
markov00 authored May 24, 2018
1 parent 1683909 commit e662b48
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 9 deletions.
20 changes: 11 additions & 9 deletions src/ui/public/markdown/markdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,8 @@ export class Markdown extends Component {
this.markdownIt = markdownFactory(this.props.whiteListedRules, this.props.openLinksInNewTab);

this.state = {
renderedMarkdown: this.transformMarkdown(this.props)
renderedMarkdown: this.transformMarkdown(this.props),
};


}

/**
Expand All @@ -68,9 +66,16 @@ export class Markdown extends Component {
}

componentWillReceiveProps(props) {
if (props.markdown !== this.props.markdown) {
const hasOpenLinksInNewTabChanged = props.openLinksInNewTab !== this.props.openLinksInNewTab;
const hasMarkdownChanged = props.markdown !== this.props.markdown;
const hasWhiteListerRulesChanged = props.whiteListedRules !== this.props.whiteListedRules;

if (hasOpenLinksInNewTabChanged || hasWhiteListerRulesChanged) {
this.markdownIt = markdownFactory(props.whiteListedRules, props.openLinksInNewTab);
}
if (hasMarkdownChanged || hasOpenLinksInNewTabChanged || hasWhiteListerRulesChanged) {
this.setState({
renderedMarkdown: this.transformMarkdown(props)
renderedMarkdown: this.transformMarkdown(props),
});
}
}
Expand All @@ -84,10 +89,7 @@ export class Markdown extends Component {
...rest
} = this.props;

const classes = classNames(
'markdown-body',
className
);
const classes = classNames('markdown-body', className);

return (
<div
Expand Down
23 changes: 23 additions & 0 deletions src/ui/public/markdown/markdown.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,27 @@ describe('props', () => {
/>);
expect(component).toMatchSnapshot(); // eslint-disable-line
});

test('should update markdown when openLinksInNewTab prop change', () => {
const component = shallow(<Markdown
markdown={markdown}
openLinksInNewTab={false}
/>);
expect(component.render().find('a').prop('target')).not.toBe('_blank');
component.setProps({ openLinksInNewTab: true });
expect(component.render().find('a').prop('target')).toBe('_blank');
});

test('should update markdown when whiteListedRules prop change', () => {
const markdown = '*emphasis* `backticks`';
const component = shallow(<Markdown
markdown={markdown}
whiteListedRules={['emphasis', 'backticks']}
/>);
expect(component.render().find('em')).toHaveLength(1);
expect(component.render().find('code')).toHaveLength(1);
component.setProps({ whiteListedRules: ['backticks'] });
expect(component.render().find('code')).toHaveLength(1);
expect(component.render().find('em')).toHaveLength(0);
});
});

0 comments on commit e662b48

Please sign in to comment.