[Vis: Default editor] EUIficate Markdown tab #42677

maryia-lapata · 2019-08-06T11:21:19Z

Summary

A part of #38273.
EUIfication of the Options tab in the Markdown vis.

Before	After

Error message:

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-08-06T11:21:21Z

Pinging @elastic/kibana-app

maryia-lapata · 2019-08-06T11:51:10Z

src/legacy/core_plugins/vis_type_markdown/public/markdown_vis_options.tsx

+ rows={20}
+ fullWidth={true}
+ data-test-subj="markdownTextarea"
+ // resize="none"


I'll uncomment this when elastic/eui#2201 is resolved.

elasticmachine · 2019-08-06T14:33:19Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cchaos

Just a couple suggestions for enhancements.

src/legacy/core_plugins/vis_type_markdown/public/markdown_vis_options.tsx

elasticmachine · 2019-08-08T10:53:23Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

maryia-lapata · 2019-08-08T10:55:51Z

src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/range.tsx

- value={value}
- onChange={ev => setValue(paramName, ev.target.valueAsNumber)}
+ value={stateValue}
+ // @ts-ignore


I'll uncomment this once elastic/eui#2211 is available.

elasticmachine · 2019-08-08T11:39:10Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cchaos

One more suggestion since the popout icon is so big compared to the text.

Other than that, LGTM

src/legacy/core_plugins/vis_type_markdown/public/markdown_options.tsx

sulemanof · 2019-08-08T13:11:45Z

src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/range.tsx

+ });
+
+ const onChangeHandler = (
+ { target: { valueAsNumber } }: React.ChangeEvent<HTMLInputElement>,


Don't forget to remove this type after add definition in EUI

src/legacy/core_plugins/vis_type_markdown/public/markdown_options.tsx

maryia-lapata · 2019-08-08T14:57:04Z

src/legacy/core_plugins/vis_type_markdown/public/markdown_options.tsx

+import { RangeOption } from '../../kbn_vislib_vis_types/public/controls/range';
+import { MarkdownVisParams } from './markdown_fn';
+
+function MarkdownOptions({ stateParams, setValue }: VisOptionsProps<MarkdownVisParams>) {


Sure. done.

elasticmachine · 2019-08-12T07:54:19Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

maryia-lapata · 2019-08-12T08:24:09Z

@flash1293, @timroes, @wylieconlon could someone please review this PR?

timroes

I was wondering, since that panel kind of looks very cluttered compared to our other visualizations, if it would make sense to split up the actual "Markdown" panel and create another panel under that with "Styling" or "Options" that would contain the font and links settings? @cchaos your thoughts on that?

src/legacy/core_plugins/vis_type_markdown/public/markdown_options.tsx

cchaos · 2019-08-12T14:14:52Z

@timroes A second panel works for me!

maryia-lapata · 2019-08-12T14:22:39Z

@timroes, @cchaos let me merge this PR and make a second panel in a separate PR 🙂.

timroes

In that case PR looks good to me. Tested on Chrome Linux

elasticmachine · 2019-08-12T15:27:44Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* Migrate Markdown tab * Code review comments * Apply TS * Add validation for Range control * Apply comments * Move type to a separate file * Update markdown_options.tsx

…p-metrics-selectall * 'master' of github.com:elastic/kibana: (27 commits) [ML] Data Frames: Analytics job creation. (elastic#43102) [Vis Default editor] Fix issue with Rollup (elastic#42430) [Vis: Default editor] EUIficate Markdown tab (elastic#42677) [New Platform Migration Phase I]: update dateHisogramInterval & parseEsInterval imports (elastic#42917) [Infra UI] Add AWS metrics to node detail page (elastic#42153) update apm index pattern (elastic#43106) [SIEM] Toggle Column / Code Coverage and Cypress (elastic#42766) skip failing test (elastic#43163) [code] Add option to turn the go dependency download on/off. (elastic#43096) disable visual regression jobs Removed dead code (elastic#42774) fixes csv export of saved searches that have _source field (elastic#43123) Export missing Context types (elastic#43051) Update dependency supports-color to v7 (elastic#43064) switch to icon type string instead of node (elastic#43111) [Maps] Enable borders for icon symbols (elastic#43066) [ftr] enable visualRegression jobs (elastic#42989) [ML] Converting single to multi metric job (elastic#42532) fix(NA): dont clean dll module if it is a package json file (elastic#42904) [Logs UI] Add link from the sample web logs to the Logs UI (elastic#42635) ...

* Migrate Markdown tab * Code review comments * Apply TS * Add validation for Range control * Apply comments * Move type to a separate file * Update markdown_options.tsx

maryia-lapata added 2 commits August 6, 2019 12:22

Migrate Markdown tab

872d2dc

Merge branch 'master' into markdown-options

0940425

maryia-lapata added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Vis Editor Visualization editor issues v8.0.0 v7.4.0 labels Aug 6, 2019

maryia-lapata requested review from cchaos and sulemanof August 6, 2019 11:21

maryia-lapata requested a review from a team as a code owner August 6, 2019 11:21

maryia-lapata added the release_note:skip Skip the PR/issue when compiling release notes label Aug 6, 2019

maryia-lapata commented Aug 6, 2019

View reviewed changes

timroes mentioned this pull request Aug 6, 2019

EUIficate visualization option panels #38273

Closed

10 tasks

This comment has been minimized.

Sign in to view

cchaos reviewed Aug 6, 2019

View reviewed changes

maryia-lapata added 2 commits August 7, 2019 11:21

Merge branch 'master' into markdown-options

fde4dde

Code review comments

340c720

This comment has been minimized.

Sign in to view

sulemanof reviewed Aug 7, 2019

View reviewed changes

src/legacy/core_plugins/vis_type_markdown/public/markdown_vis_options.tsx Outdated Show resolved Hide resolved

maryia-lapata added 2 commits August 8, 2019 12:36

Merge branch 'master' into markdown-options

d4bcdd5

Apply TS

1228bcf

maryia-lapata requested review from sulemanof and cchaos August 8, 2019 09:49

Add validation for Range control

bba9e2e

maryia-lapata commented Aug 8, 2019

View reviewed changes

cchaos approved these changes Aug 8, 2019

View reviewed changes

src/legacy/core_plugins/vis_type_markdown/public/markdown_options.tsx Outdated Show resolved Hide resolved

sulemanof reviewed Aug 8, 2019

View reviewed changes

src/legacy/core_plugins/vis_type_markdown/public/markdown_options.tsx Show resolved Hide resolved

sulemanof approved these changes Aug 8, 2019

View reviewed changes

Apply comments

1f929c0

maryia-lapata requested review from wylieconlon and flash1293 August 8, 2019 13:20

timroes reviewed Aug 8, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

Move type to a separate file

a23116e

This comment has been minimized.

Sign in to view

maryia-lapata requested a review from timroes August 9, 2019 07:42

Merge branch 'master' of github.com:elastic/kibana into markdown-options

547bb86

timroes reviewed Aug 12, 2019

View reviewed changes

src/legacy/core_plugins/vis_type_markdown/public/markdown_options.tsx Outdated Show resolved Hide resolved

Update markdown_options.tsx

5df3ac6

timroes approved these changes Aug 12, 2019

View reviewed changes

maryia-lapata merged commit c1acf71 into elastic:master Aug 13, 2019

maryia-lapata deleted the markdown-options branch August 13, 2019 07:48

maryia-lapata mentioned this pull request Aug 13, 2019

[7.x] [Vis: Default editor] EUIficate Markdown tab (#42677) #43171

Merged

maryia-lapata added the backported label Aug 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Vis: Default editor] EUIficate Markdown tab #42677

[Vis: Default editor] EUIficate Markdown tab #42677

maryia-lapata commented Aug 6, 2019 •

edited

Loading

elasticmachine commented Aug 6, 2019

maryia-lapata Aug 6, 2019

This comment has been minimized.

elasticmachine commented Aug 6, 2019

cchaos left a comment

This comment has been minimized.

This comment has been minimized.

elasticmachine commented Aug 8, 2019

maryia-lapata Aug 8, 2019

elasticmachine commented Aug 8, 2019

cchaos left a comment

sulemanof Aug 8, 2019

This comment was marked as resolved.

maryia-lapata Aug 8, 2019

This comment has been minimized.

This comment has been minimized.

elasticmachine commented Aug 12, 2019

maryia-lapata commented Aug 12, 2019

timroes left a comment

cchaos commented Aug 12, 2019

maryia-lapata commented Aug 12, 2019

timroes left a comment

elasticmachine commented Aug 12, 2019

[Vis: Default editor] EUIficate Markdown tab #42677

[Vis: Default editor] EUIficate Markdown tab #42677

Conversation

maryia-lapata commented Aug 6, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Aug 6, 2019

maryia-lapata Aug 6, 2019

Choose a reason for hiding this comment

This comment has been minimized.

elasticmachine commented Aug 6, 2019

💚 Build Succeeded

cchaos left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

elasticmachine commented Aug 8, 2019

💚 Build Succeeded

maryia-lapata Aug 8, 2019

Choose a reason for hiding this comment

elasticmachine commented Aug 8, 2019

💚 Build Succeeded

cchaos left a comment

Choose a reason for hiding this comment

sulemanof Aug 8, 2019

Choose a reason for hiding this comment

This comment was marked as resolved.

maryia-lapata Aug 8, 2019

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

elasticmachine commented Aug 12, 2019

💚 Build Succeeded

maryia-lapata commented Aug 12, 2019

timroes left a comment

Choose a reason for hiding this comment

cchaos commented Aug 12, 2019

maryia-lapata commented Aug 12, 2019

timroes left a comment

Choose a reason for hiding this comment

elasticmachine commented Aug 12, 2019

💚 Build Succeeded

maryia-lapata commented Aug 6, 2019 •

edited

Loading