From e662b48d4aaae02170c9a7719e6e78e57ed075a0 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Thu, 24 May 2018 12:03:42 +0200 Subject: [PATCH] Fix rendering markdown when updating open in new tab setting (#19356) * 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 --- src/ui/public/markdown/markdown.js | 20 +++++++++++--------- src/ui/public/markdown/markdown.test.js | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/ui/public/markdown/markdown.js b/src/ui/public/markdown/markdown.js index 2e2b47e37c7bc..d34a387af4cdf 100644 --- a/src/ui/public/markdown/markdown.js +++ b/src/ui/public/markdown/markdown.js @@ -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), }; - - } /** @@ -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), }); } } @@ -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 (
{ />); expect(component).toMatchSnapshot(); // eslint-disable-line }); + + test('should update markdown when openLinksInNewTab prop change', () => { + const component = shallow(); + 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(); + 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); + }); });