[Vis: Default Editor] Euificate table options tab #46013

maryia-lapata · 2019-09-18T14:54:30Z

Summary

EUIfication of options tab for Data Table vis.
Part of #38273, #16639.

This PR contains:

Euification of table-vis-params directive;
types for TableVisParams;
move _get_columns.js to TS;
SwitchOption: rename dataTestSubj to data-test-subj.

Screenshots:

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-09-24T11:52:21Z

Pinging @elastic/kibana-app

maryia-lapata · 2019-09-25T14:29:12Z

@elasticmachine merge upstream

cchaos

Just one small text change, otherwise LGTM

src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx

maryia-lapata · 2019-09-26T07:41:38Z

src/legacy/core_plugins/vis_type_table/public/table_vis_params.js

-        function () {
-          if (!$scope.vis) return;
-          const params = $scope.editorState.params;
-          $scope.metricsAtAllLevels = params.showPartialRows || params.showMetricsAtAllLevels;


I didn't find a place where metricsAtAllLevels variable is read. Moreover there is the same logic in vis type definition - hierarchicalData method. So I didn't migrate this code.

elasticmachine · 2019-09-26T08:53:51Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

sulemanof · 2019-09-26T15:06:43Z

src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx

+          defaultMessage: 'Rows per page',
+        })}
+        paramName="perPage"
+        value={stateParams.perPage}


I suppose this value could have a min bound 1 with validation also

elasticmachine · 2019-09-26T16:47:23Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

src/legacy/core_plugins/vis_type_table/public/table_vis_type.ts

src/legacy/core_plugins/vis_type_table/public/components/utils.ts

src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx

elasticmachine · 2019-10-01T12:33:55Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

sulemanof

LGTM!
Tested locally on Chrome/Windows.

elasticmachine · 2019-10-02T17:35:29Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-10-03T12:04:08Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 4f89b09

sulemanof · 2019-10-07T08:27:12Z

@elasticmachine merge upstream

elasticmachine · 2019-10-07T09:33:37Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 3cbd2a7

flash1293

LGTM, two small remarks about the structure of the panel

flash1293 · 2019-10-07T16:13:26Z

src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx

+        <h3>
+          <FormattedMessage
+            id="visTypeTable.params.showMetricsLabel.optionsTitle"
+            defaultMessage="Options"


In Markdown the only panel is called "Settings" - not sure which one is better, do we even need one if there is just a single panel

Let's better remove it for consistency, like such titles are skipped in Controls, Coordinate map and Tag cloud. Such titles are actually duplicates the tab name - Options

flash1293 · 2019-10-07T16:15:07Z

src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx

+        </h3>
+      </EuiTitle>
+      <EuiSpacer size="s" />
+      <NumberInputOption


We could put a tooltip here to explain that leaving this empty means it will use the amount of buckets in the response.

Done.
@snide , please, confirm these changes.

elasticmachine · 2019-10-08T13:52:16Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 3904aea

sulemanof · 2019-10-09T07:49:51Z

@elasticmachine merge upstream

elasticmachine · 2019-10-09T09:07:48Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 1b9ee6f

* Euificate table options tab * Add validation * Use SchemaConfig for dimensions * Update snapshots

Euificate table options tab

5698504

maryia-lapata added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.5.0 labels Sep 18, 2019

This comment has been minimized.

Sign in to view

maryia-lapata added 2 commits September 23, 2019 18:08

Merge branch 'master' into data-table-tab

f51bbb9

Refactoring

d27bdff

maryia-lapata requested a review from cchaos September 24, 2019 11:43

timroes mentioned this pull request Sep 24, 2019

EUIficate visualization option panels #38273

Closed

10 tasks

maryia-lapata marked this pull request as ready for review September 24, 2019 11:51

maryia-lapata added Feature:Vis Editor Visualization editor issues Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Sep 24, 2019

maryia-lapata requested a review from sulemanof September 24, 2019 11:52

maryia-lapata changed the title ~~[Default Editor] Euificate table options tab~~ [Vis: Default Editor] Euificate table options tab Sep 24, 2019

maryia-lapata added the Feature:Data Table Data table visualization feature label Sep 24, 2019

This comment has been minimized.

Sign in to view

Merge branch 'master' into data-table-tab

0038919

This comment has been minimized.

Sign in to view

cchaos approved these changes Sep 25, 2019

View reviewed changes

src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx Outdated Show resolved Hide resolved

maryia-lapata added 2 commits September 26, 2019 10:10

Merge branch 'master' into data-table-tab

b3d79ec

Change label

f75923e

maryia-lapata commented Sep 26, 2019

View reviewed changes

Refactoring

fe667d2

maryia-lapata requested review from cchaos and myasonik September 26, 2019 07:54

sulemanof reviewed Sep 26, 2019

View reviewed changes

Add validation

e42d06e

sulemanof suggested changes Sep 27, 2019

View reviewed changes

maryia-lapata added 2 commits October 1, 2019 13:58

Merge branch 'master' into data-table-tab

4232fb0

Fix code review comments

3ddb9cf

maryia-lapata assigned sulemanof Oct 1, 2019

sulemanof added 3 commits October 2, 2019 19:08

Update table_vis_options

d62fc88

Merge remote-tracking branch 'upstream/master' into data-table-tab

b1becd2

Use SchemaConfig for dimensions

f4e50d9

sulemanof requested a review from flash1293 October 2, 2019 16:18

sulemanof approved these changes Oct 2, 2019

View reviewed changes

Update snapshots

4f89b09

Merge branch 'master' into data-table-tab

3cbd2a7

flash1293 approved these changes Oct 7, 2019

View reviewed changes

Fix comments

3904aea

sulemanof requested a review from snide October 8, 2019 12:43

Merge branch 'master' into data-table-tab

1b9ee6f

sulemanof merged commit 6a0da3d into elastic:master Oct 9, 2019

sulemanof deleted the data-table-tab branch October 9, 2019 09:19

sulemanof mentioned this pull request Oct 9, 2019

[7.x] [Vis: Default Editor] Euificate table options tab (#46013) #47666

Merged

sulemanof pushed a commit to sulemanof/kibana that referenced this pull request Oct 9, 2019

[Vis: Default Editor] Euificate table options tab (elastic#46013)

ecf1735

* Euificate table options tab * Add validation * Use SchemaConfig for dimensions * Update snapshots

sulemanof added a commit that referenced this pull request Oct 9, 2019

[Vis: Default Editor] Euificate table options tab (#46013) (#47666)

e849663

* Euificate table options tab * Add validation * Use SchemaConfig for dimensions * Update snapshots

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Vis: Default Editor] Euificate table options tab #46013

[Vis: Default Editor] Euificate table options tab #46013

maryia-lapata commented Sep 18, 2019 •

edited

Loading

This comment has been minimized.

elasticmachine commented Sep 24, 2019

This comment has been minimized.

maryia-lapata commented Sep 25, 2019

This comment has been minimized.

cchaos left a comment

maryia-lapata Sep 26, 2019

elasticmachine commented Sep 26, 2019

sulemanof Sep 26, 2019

elasticmachine commented Sep 26, 2019

elasticmachine commented Oct 1, 2019

sulemanof left a comment

elasticmachine commented Oct 2, 2019

elasticmachine commented Oct 3, 2019

sulemanof commented Oct 7, 2019

elasticmachine commented Oct 7, 2019

flash1293 left a comment

flash1293 Oct 7, 2019

sulemanof Oct 8, 2019

flash1293 Oct 7, 2019

sulemanof Oct 8, 2019

elasticmachine commented Oct 8, 2019

sulemanof commented Oct 9, 2019

elasticmachine commented Oct 9, 2019

[Vis: Default Editor] Euificate table options tab #46013

[Vis: Default Editor] Euificate table options tab #46013

Conversation

maryia-lapata commented Sep 18, 2019 • edited Loading

Summary

Screenshots:

Checklist

For maintainers

This comment has been minimized.

elasticmachine commented Sep 24, 2019

This comment has been minimized.

maryia-lapata commented Sep 25, 2019

This comment has been minimized.

cchaos left a comment

Choose a reason for hiding this comment

maryia-lapata Sep 26, 2019

Choose a reason for hiding this comment

elasticmachine commented Sep 26, 2019

💚 Build Succeeded

sulemanof Sep 26, 2019

Choose a reason for hiding this comment

elasticmachine commented Sep 26, 2019

💚 Build Succeeded

elasticmachine commented Oct 1, 2019

💔 Build Failed

sulemanof left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 2, 2019

💔 Build Failed

elasticmachine commented Oct 3, 2019

💚 Build Succeeded

sulemanof commented Oct 7, 2019

elasticmachine commented Oct 7, 2019

💚 Build Succeeded

flash1293 left a comment

Choose a reason for hiding this comment

flash1293 Oct 7, 2019

Choose a reason for hiding this comment

sulemanof Oct 8, 2019

Choose a reason for hiding this comment

flash1293 Oct 7, 2019

Choose a reason for hiding this comment

sulemanof Oct 8, 2019

Choose a reason for hiding this comment

elasticmachine commented Oct 8, 2019

💚 Build Succeeded

sulemanof commented Oct 9, 2019

elasticmachine commented Oct 9, 2019

💚 Build Succeeded

maryia-lapata commented Sep 18, 2019 •

edited

Loading