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

[data view mgmt] Delete data views from list #127557

Merged
merged 16 commits into from
Mar 21, 2022
Merged

[data view mgmt] Delete data views from list #127557

merged 16 commits into from
Mar 21, 2022

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Mar 13, 2022 •
edited by mdefazio
Loading

Summary

Addresses improved data view delete functionality as described here - #124346

Screen Shot 2022-03-16 at 6 26 25 PM

Confirm on data view detail page -
Screen Shot 2022-03-17 at 4 19 52 PM

Screen Shot 2022-03-17 at 4 20 18 PM

Screen Shot 2022-03-17 at 4 20 52 PM

Closes #124346

@mattkime mattkime changed the title mostly in place, need to update some content [data view mgmt] Delete data views from list Mar 13, 2022
@mattkime
Copy link
Contributor Author

@elasticmachine merge upstream

@mattkime mattkime marked this pull request as ready for review March 16, 2022 23:34
@mattkime mattkime requested a review from a team as a code owner March 16, 2022 23:34
@mattkime mattkime added v8.2.0 Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages Team:AppServicesSv labels Mar 16, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesSv)

@mattkime mattkime added the release_note:skip Skip the PR/issue when compiling release notes label Mar 16, 2022
shivindera
shivindera reviewed Mar 17, 2022
View reviewed changes
src/plugins/data_view_management/public/components/index_pattern_table/index_pattern_table.tsx
@@ -25,6 +26,8 @@ import { getIndexPatterns } from '../utils';
import { getListBreadcrumbs } from '../breadcrumbs';
import { SpacesList } from './spaces_list';
import type { SpacesContextProps } from '../../../../../../x-pack/plugins/spaces/public';
import { removeDataView, RemoveDataViewProps } from '../edit_index_pattern';
import { deleteModalMsg } from './delete_modal_msg';
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 this be DeleteModalMsg?

mdefazio
mdefazio requested changes Mar 17, 2022
View reviewed changes
Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

Just one nit: I think we should continue the use of the danger style button into the delete modal.

There's a 'dangerous confirm modal' example in EUI docs which I think makes sense here.

gchaps
gchaps approved these changes Mar 17, 2022
View reviewed changes
Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

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

LGTM with one minor comment.

src/plugins/data_view_management/public/components/edit_index_pattern/edit_index_pattern.tsx
) : (
<FormattedMessage
id="indexPatternManagement.editDataView.deleteWarning"
defaultMessage="The data view {dataViewName} will be deleted. You can't undo this action."
Copy link
Contributor

Choose a reason for hiding this comment

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

Another option is to put the name at the end of the sentence:

Delete the data view {dataViewName}. You can't undo this action.

Copy link
Contributor

Choose a reason for hiding this comment

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

It also removes the passive "will be deleted".

@gchaps gchaps added the ui-copy Review of UI copy with docs team is recommended label Mar 17, 2022
@mattkime
Copy link
Contributor Author

@elasticmachine merge upstream

@mattkime mattkime requested a review from mdefazio March 17, 2022 23:27
mdefazio
mdefazio approved these changes Mar 18, 2022
View reviewed changes
Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes!

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Default Accessibility Tests / Kibana tags page meets a11y validations tag management page with connections column populated meets a11y requirements

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dataViewManagement 135 137 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataViewManagement 120.9KB 123.4KB +2.6KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dataViewManagement 4.3KB 4.3KB +50.0B

History

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

@mattkime mattkime merged commit a6ec64f into elastic:main Mar 21, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 127557 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 23, 2022
@mdefazio mdefazio mentioned this pull request Mar 24, 2022
Closed
2 tasks
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 127557 or prevent reminders by adding the backport:skip label.

4 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 127557 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 127557 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 127557 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 127557 or prevent reminders by adding the backport:skip label.

@mattkime mattkime added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Mar 30, 2022
@Dosant Dosant mentioned this pull request Apr 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdefazio mdefazio mdefazio approved these changes

@shivindera shivindera shivindera approved these changes

@gchaps gchaps gchaps approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages release_note:skip Skip the PR/issue when compiling release notes ui-copy Review of UI copy with docs team is recommended v8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Data Views] UX: Improve sharing across spaces
7 participants
@mattkime @elasticmachine @kibana-ci @kibanamachine @mdefazio @shivindera @gchaps