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.

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

[Enterprise Search] Fix index delete modal #142447

Merged

Conversation

navarone-feekery
Copy link
Contributor

@navarone-feekery navarone-feekery commented Oct 3, 2022

Summary

https://github.com/elastic/enterprise-search-team/issues/2835

  • Disable Delete button on index delete modal while process is running
  • Change Cancel text to Close during process
    • functionality remains the same (modal is closed), but the delete process cannot be cancelled so this is purely UX

Checklist

Delete any items that are not applicable to this PR.

Comment on lines +41 to +46
? i18n.translate(
'xpack.enterpriseSearch.content.searchIndices.deleteModal.closeButton.title',
{
defaultMessage: 'Close',
}
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this enough for translations?

Copy link
Contributor Author

@navarone-feekery navarone-feekery Oct 3, 2022

Choose a reason for hiding this comment

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

xpack.* seems to have no translation files (even an en.json) so is it possible we don't translate this?

Copy link
Member

Choose a reason for hiding this comment

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

This is good enough for translations, there's no xpack-specific translation files, there's just giant translation files for everything in Kibana. English is the default message so that doesn't get a translation file.

@navarone-feekery navarone-feekery marked this pull request as ready for review October 3, 2022 10:00
@navarone-feekery navarone-feekery requested a review from a team October 3, 2022 10:00
@navarone-feekery navarone-feekery added auto-backport Deprecated - use backport:version if exact versions are needed v8.5.0 v8.6.0 labels Oct 3, 2022
@navarone-feekery navarone-feekery changed the title Fix index delete modal [Enterprise Search] Fix index delete modal Oct 3, 2022
@navarone-feekery navarone-feekery added the release_note:skip Skip the PR/issue when compiling release notes label Oct 3, 2022
@navarone-feekery
Copy link
Contributor Author

@elasticmachine merge upstream

Comment on lines +41 to +46
? i18n.translate(
'xpack.enterpriseSearch.content.searchIndices.deleteModal.closeButton.title',
{
defaultMessage: 'Close',
}
)
Copy link
Member

Choose a reason for hiding this comment

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

This is good enough for translations, there's no xpack-specific translation files, there's just giant translation files for everything in Kibana. English is the default message so that doesn't get a translation file.

@@ -72,8 +72,10 @@ export interface IndicesValues {
deleteModalIndex: ElasticsearchViewIndex | null;
deleteModalIndexName: string;
deleteModalIngestionMethod: IngestionMethod;
deleteStatus: typeof DeleteIndexApiLogic.values.status;
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: this should just be typed as Status, which is already imported in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sphilipse on L84 we also have status: typeof FetchIndicesAPILogic.values.status;. Should that also be changed to status: Status;?

Copy link
Member

Choose a reason for hiding this comment

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

Probably yes! Not blocking either way.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
enterpriseSearch 1.7MB 1.7MB +251.0B

History

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

@navarone-feekery navarone-feekery merged commit cc26353 into elastic:main Oct 5, 2022
@navarone-feekery navarone-feekery deleted the fix-index-delete-modal branch October 5, 2022 09:26
kibanamachine pushed a commit that referenced this pull request Oct 5, 2022
* Disable index delete button during process

* Change cancel to close during delete process

(cherry picked from commit cc26353)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

kibanamachine added a commit that referenced this pull request Oct 5, 2022
* Disable index delete button during process

* Change cancel to close during delete process

(cherry picked from commit cc26353)

Co-authored-by: Nav <13634519+navarone-feekery@users.noreply.github.com>
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022
* Disable index delete button during process

* Change cancel to close during delete process
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022
* Disable index delete button during process

* Change cancel to close during delete process
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:EnterpriseSearch v8.5.0 v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants