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.

Sign up for GitHub

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

Jump to bottom

adding markdown vis renderer #75532

Merged
merged 2 commits into from
Aug 21, 2020
Merged

adding markdown vis renderer #75532

merged 2 commits into from
Aug 21, 2020

Conversation

ppisljar
Copy link
Member

Summary

adds markdown vis renderer and toExpressionAST function

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@ppisljar ppisljar added Feature:Markdown Markdown visualization feature Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Aug 20, 2020
@ppisljar ppisljar requested a review from a team August 20, 2020 08:41
@ppisljar ppisljar requested a review from a team as a code owner August 20, 2020 08:41
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

flash1293
flash1293 approved these changes Aug 20, 2020
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in Firefox, works as expected (viewing visualization in Visualize, in Dashboard and in reporting pdf).

Noticed a typo in the test, if that's fixed this LGTM.

src/plugins/vis_type_markdown/public/to_ast.test.ts Outdated
});

it('with params', () => {
vis.params = { markdown: '### my markdown', font: 15, openLinksInNewTab: true };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be fontSize instead of font? The 15 is not showing up in the snapshot as well.

lukeelmers
lukeelmers approved these changes Aug 20, 2020
View reviewed changes
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, pending 💚 from CI

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
visTypeMarkdown 258 +2 256

async chunks size

id value diff baseline
visTypeMarkdown 40.7KB -1.0B 40.7KB

page load bundle size

id value diff baseline
visTypeMarkdown 556.3KB +3.1KB 553.2KB
visualizations 408.0KB -456.0B 408.5KB
total +2.7KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@ppisljar ppisljar merged commit 3ddc2ac into elastic:master Aug 21, 2020
@ppisljar ppisljar mentioned this pull request Aug 21, 2020
Merged
ppisljar added a commit to ppisljar/kibana that referenced this pull request Aug 21, 2020
@ppisljar
adding markdown vis renderer (elastic#75532)
cbf8214
ppisljar added a commit that referenced this pull request Aug 21, 2020
@ppisljar
adding markdown vis renderer (#75532) (#75628)
4fdcf6b
thomasneirynck pushed a commit to thomasneirynck/kibana that referenced this pull request Aug 21, 2020
@ppisljar @thomasneirynck
adding markdown vis renderer (elastic#75532)
6e162ff
@timroes timroes mentioned this pull request Sep 15, 2020
Closed
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flash1293 flash1293 flash1293 approved these changes

@lukeelmers lukeelmers lukeelmers approved these changes

Assignees
No one assigned
Labels
Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Feature:Markdown Markdown visualization feature release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ppisljar @elasticmachine @kibanamachine @flash1293 @lukeelmers