Move table-list-view to kibana-react #50046

maryia-lapata · 2019-11-06T13:04:10Z

Summary

Move table_list_view to kibana_react

Fixes #49599

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-11-06T13:04:14Z

Pinging @elastic/kibana-app (Team:KibanaApp)

elasticmachine · 2019-11-06T14:15:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: f76b6e4

flash1293 · 2019-11-06T23:28:39Z

src/legacy/core_plugins/kibana/public/dashboard/saved_dashboard/saved_dashboard.d.ts

@@ -25,7 +25,7 @@ import { Query } from 'src/legacy/core_plugins/data/public';
 import { Filter } from '@kbn/es-query';

 export interface SavedObjectDashboard extends SavedObject {
-  id?: string;
+  id: string;


Did you change this because of the map() in the list view? I think it is possible that there is a SavedObjectDashboard around without an id (in cases where the dashboard isn't saved yet), so I would like to not change the type here. You can filter out the dashboards without id to stop typescript from complaining (it won't be a runtime problem because there won't be unsaved dashboards in the listing even though that might change in the future):

selectedIds: obj.map(item => item.id).filter((id: undefined | string): id is string => Boolean(id))

flash1293 · 2019-11-06T23:29:14Z

src/legacy/ui/public/saved_objects/saved_object.d.ts

@@ -26,5 +26,5 @@ export interface SaveOptions {
 export interface SavedObject {
  save: (saveOptions: SaveOptions) => Promise<string>;
  copyOnSave: boolean;
-  id?: string;
+  id: string;


Same thing here, see above

flash1293 · 2019-11-06T23:32:51Z

src/plugins/kibana_react/public/table_list_view/table_list_view.tsx

      await this.props.deleteItems(this.state.selectedIds.map(id => itemsById[id]));
    } catch (error) {
-      toastNotifications.addDanger({
+      npStart.core.notifications.toasts.addDanger({


core.notifications should either get passed in as a prop or be pulled out of the useKibana() hook. In both cases you have to make sure the places where the component is used do pass the core services in correctly. AFAIK it's used in visualize, dashboard, graph.

flash1293 · 2019-11-06T23:35:18Z

x-pack/plugins/translations/translations/ja-JP.json

@@ -658,6 +658,20 @@
    "core.euiSuperUpdateButton.refreshButtonLabel": "更新",
    "core.euiSuperUpdateButton.updateButtonLabel": "更新",
    "core.euiSuperUpdateButton.updatingButtonLabel": "更新中",
+    "kibana-react.table_list_view.listing.deleteButtonMessage": "{itemCount} 件の {entityName} を削除",


I think the ids should use camel case: kibana-react.tableListView.listing.deleteButtonMessage

flash1293 · 2019-11-06T23:38:21Z

src/plugins/kibana_react/public/table_list_view/table_list_view.tsx

-  initialFilter: EMPTY_FILTER,
-};
-
-export const TableListView = injectI18n(TableListViewUi);


We should wrap the render tree into an I18nProvider if we are removing this - I haven't checked but it looks like there is no I18nProvider anymore for the dashboard listing. But even if not it's easier to use with an I18nProvider.

@maryia-lapata Did you address this comment somewhere?

Not yet, I'll do it today.

@flash1293 I checked this PR with the zh-CN locale and as you can see on the screenshot the labels are localized:

injectI18n is HOC that just passes intl object into props.
The Dashboard listing is already wrapped into i18nContext here https://github.com/maryia-lapata/kibana/blob/master/src/legacy/core_plugins/kibana/public/dashboard/index.js#L53 . So it's no need to do it again.

streamich · 2019-11-09T11:29:01Z

src/plugins/kibana_react/public/table_list_view/table_list_view.tsx

 import { npStart } from 'ui/new_platform';
+import { SavedObject } from 'ui/saved_objects/saved_object';


These two ui/* imports are not allowed here. You can pass those to the component through props or context instread.

elasticmachine · 2019-11-11T10:07:38Z

💔 Build Failed

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

streamich

App Arch changes LGTM

elasticmachine · 2019-11-11T15:32:44Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: c2706a1

elasticmachine · 2019-11-11T17:06:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: e841832

flash1293

LGTM, did not test locally. If the i18n thing is solved and I just didn't get how this can be merged.

* upstream/master: Move table-list-view to kibana-react (elastic#50046) [ML] Stats bar for data frame analytics (elastic#49464) [ML] Make navigation in tests more stable (elastic#50132) Migrate authorization subsystem to the new platform. (elastic#46145) Bugfix: Interpreter conversion of string to number should throw on NaN elastic#27788 (elastic#50063) Update dependency @elastic/charts to v14 (elastic#49947) [ML] Adding cloud specific ML node warning (elastic#50139) Fixing bugs in the Shareable Runtime (elastic#49965) Revert router base name for Uptime plugin to use hash in default path. (elastic#50095) Ability to have telemetry always opted in (elastic#49798) Add "Get Help" and "Kibana Feedback" links to the help popover (elastic#49797) Removes references to Elasticsearch mapping types (elastic#47610) [skip-ci] Replace coordinate map in Kibana getting started docs with Maps (elastic#50167) [ML] Indicate missing required privileges for import in File Data Viz (elastic#50147) [SIEM][Detection Engine] Removes technical debt and minor bug fixes (elastic#50111)

…skip ci] * upstream/master: Move table-list-view to kibana-react (elastic#50046) [ML] Stats bar for data frame analytics (elastic#49464) [ML] Make navigation in tests more stable (elastic#50132) Migrate authorization subsystem to the new platform. (elastic#46145) Bugfix: Interpreter conversion of string to number should throw on NaN elastic#27788 (elastic#50063) Update dependency @elastic/charts to v14 (elastic#49947) [ML] Adding cloud specific ML node warning (elastic#50139) Fixing bugs in the Shareable Runtime (elastic#49965) Revert router base name for Uptime plugin to use hash in default path. (elastic#50095) Ability to have telemetry always opted in (elastic#49798) Add "Get Help" and "Kibana Feedback" links to the help popover (elastic#49797) Removes references to Elasticsearch mapping types (elastic#47610) [skip-ci] Replace coordinate map in Kibana getting started docs with Maps (elastic#50167) [ML] Indicate missing required privileges for import in File Data Viz (elastic#50147) [SIEM][Detection Engine] Removes technical debt and minor bug fixes (elastic#50111)

…-fallback * 'master' of github.com:elastic/kibana: Remove internal platform types exports (elastic#50427) [APM] Document `apm_oss.metricsIndices` and `apm_oss.sourcemap… (elastic#50312) [Telemetry] Server side fetcher (elastic#50015) [SIEM] Detection engine placeholders (elastic#50220) [Uptime] Donut chart loader position centered vertically (elastic#50219) update telemetry banner notice text (elastic#50403) Fix aborting when searching without batching (elastic#49966) [Telemetry] Remove telemetry splash page and add conditional messaging (elastic#50189) Revert chromedriver update (elastic#50324) Remove deprecated argument include_type_name from ES calls (elastic#50285) [Maps] add settings to maps telemetry (elastic#50161) remove visualize loader (elastic#46910) Fix misuse of react-router and react-router-dom (elastic#50120) Move table-list-view to kibana-react (elastic#50046) [ML] Stats bar for data frame analytics (elastic#49464)

…ger-ace-theme * 'master' of github.com:elastic/kibana: (56 commits) [ML] Server info service refactor (elastic#50302) Remove internal platform types exports (elastic#50427) [APM] Document `apm_oss.metricsIndices` and `apm_oss.sourcemap… (elastic#50312) [Telemetry] Server side fetcher (elastic#50015) [SIEM] Detection engine placeholders (elastic#50220) [Uptime] Donut chart loader position centered vertically (elastic#50219) update telemetry banner notice text (elastic#50403) Fix aborting when searching without batching (elastic#49966) [Telemetry] Remove telemetry splash page and add conditional messaging (elastic#50189) Revert chromedriver update (elastic#50324) Remove deprecated argument include_type_name from ES calls (elastic#50285) [Maps] add settings to maps telemetry (elastic#50161) remove visualize loader (elastic#46910) Fix misuse of react-router and react-router-dom (elastic#50120) Move table-list-view to kibana-react (elastic#50046) [ML] Stats bar for data frame analytics (elastic#49464) [ML] Make navigation in tests more stable (elastic#50132) Migrate authorization subsystem to the new platform. (elastic#46145) Bugfix: Interpreter conversion of string to number should throw on NaN elastic#27788 (elastic#50063) Update dependency @elastic/charts to v14 (elastic#49947) ...

* Move table-list-view to kibana-react * Pass ui/* import as props * Update i18n message ids * Fix TS * Fix unit tests

Move table-list-view to kibana-react

f76b6e4

maryia-lapata added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.6.0 labels Nov 6, 2019

maryia-lapata requested a review from a team November 6, 2019 13:04

maryia-lapata requested a review from a team as a code owner November 6, 2019 13:04

flash1293 requested changes Nov 6, 2019

View reviewed changes

streamich suggested changes Nov 9, 2019

View reviewed changes

maryia-lapata added 3 commits November 11, 2019 10:18

Merge remote-tracking branch 'kibana/master' into table-list-view

b54a627

Pass ui/* import as props

33a2cd5

Update i18n message ids

3e82736

Fix TS

c2706a1

streamich approved these changes Nov 11, 2019

View reviewed changes

Fix unit tests

e841832

flash1293 approved these changes Nov 12, 2019

View reviewed changes

maryia-lapata merged commit 3a8e65a into elastic:master Nov 12, 2019

maryia-lapata deleted the table-list-view branch November 12, 2019 14:18

maryia-lapata mentioned this pull request Nov 12, 2019

[7.x] Move table-list-view to kibana-react (#50046) #50310

Merged

maryia-lapata added a commit that referenced this pull request Nov 14, 2019

Move table-list-view to kibana-react (#50046) (#50310)

2fe872b

* Move table-list-view to kibana-react * Pass ui/* import as props * Update i18n message ids * Fix TS * Fix unit tests

maryia-lapata added the backported label Nov 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move table-list-view to kibana-react #50046

Move table-list-view to kibana-react #50046

maryia-lapata commented Nov 6, 2019

elasticmachine commented Nov 6, 2019

elasticmachine commented Nov 6, 2019

flash1293 Nov 6, 2019

flash1293 Nov 6, 2019

flash1293 Nov 6, 2019

flash1293 Nov 6, 2019

flash1293 Nov 6, 2019

flash1293 Nov 12, 2019 •

edited

Loading

maryia-lapata Nov 12, 2019

maryia-lapata Nov 12, 2019 •

edited

Loading

streamich Nov 9, 2019

elasticmachine commented Nov 11, 2019

streamich left a comment

elasticmachine commented Nov 11, 2019

elasticmachine commented Nov 11, 2019

flash1293 left a comment

		import { npStart } from 'ui/new_platform';
		import { SavedObject } from 'ui/saved_objects/saved_object';

Move table-list-view to kibana-react #50046

Move table-list-view to kibana-react #50046

Conversation

maryia-lapata commented Nov 6, 2019

Summary

Checklist

For maintainers

elasticmachine commented Nov 6, 2019

elasticmachine commented Nov 6, 2019

💚 Build Succeeded

flash1293 Nov 6, 2019

Choose a reason for hiding this comment

flash1293 Nov 6, 2019

Choose a reason for hiding this comment

flash1293 Nov 6, 2019

Choose a reason for hiding this comment

flash1293 Nov 6, 2019

Choose a reason for hiding this comment

flash1293 Nov 6, 2019

Choose a reason for hiding this comment

flash1293 Nov 12, 2019 • edited Loading

Choose a reason for hiding this comment

maryia-lapata Nov 12, 2019

Choose a reason for hiding this comment

maryia-lapata Nov 12, 2019 • edited Loading

Choose a reason for hiding this comment

streamich Nov 9, 2019

Choose a reason for hiding this comment

elasticmachine commented Nov 11, 2019

💔 Build Failed

streamich left a comment

Choose a reason for hiding this comment

elasticmachine commented Nov 11, 2019

💔 Build Failed

elasticmachine commented Nov 11, 2019

💚 Build Succeeded

flash1293 left a comment

Choose a reason for hiding this comment

flash1293 Nov 12, 2019 •

edited

Loading

maryia-lapata Nov 12, 2019 •

edited

Loading