[Vis: Default editor] EUIficate Markdown tab by maryia-lapata · Pull Request #42677 · elastic/kibana

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) ...

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 Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// 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 cchaos and sulemanof 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 flash1293 and wylieconlon 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

Conversation

maryia-lapata commented Aug 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Aug 6, 2019

Uh oh!

maryia-lapata Aug 6, 2019

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

elasticmachine commented Aug 6, 2019

💚 Build Succeeded

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

elasticmachine commented Aug 8, 2019

💚 Build Succeeded

Uh oh!

maryia-lapata Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Aug 8, 2019

💚 Build Succeeded

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sulemanof Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

maryia-lapata Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

elasticmachine commented Aug 12, 2019

💚 Build Succeeded

Uh oh!

maryia-lapata commented Aug 12, 2019

Uh oh!

timroes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cchaos commented Aug 12, 2019

Uh oh!

maryia-lapata commented Aug 12, 2019

Uh oh!

timroes left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Aug 12, 2019

💚 Build Succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

maryia-lapata commented Aug 6, 2019 •

edited

Loading