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

[APM] Fix loading message missing or inconsistent in various list views #110772

Merged
merged 5 commits into from
Sep 7, 2021
Merged

[APM] Fix loading message missing or inconsistent in various list views #110772

merged 5 commits into from
Sep 7, 2021

Conversation

MiriamAparicio
Copy link
Contributor

@MiriamAparicio MiriamAparicio commented Sep 1, 2021 •
edited
Loading

Closes #110204
Closes #87034

Summary

Some table lists were not align on the loading state, they were showing different messages or loading spinners.

What was done

  • Align Loading... message on Services, Traces and Backend lists
  • Make no items message consistence on the 3 tables Services, Traces and Backed lists
  • Fix the Loading... message on the overview tables

Checklist

Delete any items that are not applicable to this PR.

@MiriamAparicio MiriamAparicio force-pushed the align-loading-state-on-tables branch from 24f94ed to 3bba876 Compare September 3, 2021 10:16
@MiriamAparicio MiriamAparicio marked this pull request as ready for review September 3, 2021 14:24
@MiriamAparicio MiriamAparicio requested a review from a team as a code owner September 3, 2021 14:24
@MiriamAparicio MiriamAparicio added v7.16.0 release_note:fix auto-backport Deprecated - use backport:version if exact versions are needed labels Sep 3, 2021
@MiriamAparicio MiriamAparicio force-pushed the align-loading-state-on-tables branch from 3bba876 to 287f5c3 Compare September 3, 2021 14:27
@botelastic botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Sep 3, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

smith
smith approved these changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

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

Looks great. Thank you!

...lugins/apm/public/components/app/service_overview/service_overview_instances_table/index.tsx Outdated
>
<EuiBasicTable
noItemsMessage={
isLoading
? i18n.translate('xpack.apm.serviceOverview.loadingText', {
defaultMessage: 'No instances found',
defaultMessage: 'Loading...',
Copy link
Contributor

Choose a reason for hiding this comment

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

Typography nit: Use an ellipsis … (option+; on a Mac) and not three periods ...

Suggested change
defaultMessage: 'Loading...',
defaultMessage: 'Loading…',

x-pack/plugins/apm/public/components/shared/managed_table/index.tsx Outdated
const showNoItemsMessage = useMemo(() => {
return isLoading
? i18n.translate('xpack.apm.managedTable.loading', {
defaultMessage: 'Loading...',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'Loading...',
defaultMessage: 'Loading…',

x-pack/plugins/apm/public/components/shared/managed_table/index.tsx Outdated
@@ -127,16 +127,18 @@ function UnoptimizedManagedTable<T>(props: Props<T>) {
};
}, [hidePerPageOptions, items, page, pageSize, pagination]);

const showNoItemsMessage = useMemo(() => {
return isLoading
? i18n.translate('xpack.apm.managedTable.loading', {
Copy link
Contributor

Choose a reason for hiding this comment

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

This i18n key was here below previously, but we should name these following the Kibana i18n guidelines so I guess this could end with loadingDescription.

Probably not necessary to change this, but I wanted to link to the guidelines so you can bookmark them.

If any i18n keys are removed, you do need to run node scripts/i18n_check --fix on them to remove them from the JSON translation files.

@MiriamAparicio MiriamAparicio force-pushed the align-loading-state-on-tables branch from 287f5c3 to 5fddbcf Compare September 6, 2021 09:04
@kibanamachine
Copy link
Contributor

💚 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
apm 4.4MB 4.4MB +574.0B

History

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

@MiriamAparicio MiriamAparicio merged commit 9b41b3f into elastic:master Sep 7, 2021
@MiriamAparicio MiriamAparicio deleted the align-loading-state-on-tables branch September 7, 2021 07:35
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 7, 2021
@MiriamAparicio @kibanamachine
[APM] Fix loading message missing or inconsistent in various list vie…
54f1ecb 
…ws (elastic#110772)

* [APM] Fix loading message missing or inconsistent in various list views

* fix types and i18n

* fix comment

* PR review comments

* fix JVM loading message
@kibanamachine kibanamachine mentioned this pull request Sep 7, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Sep 7, 2021
@kibanamachine @MiriamAparicio
[APM] Fix loading message missing or inconsistent in various list vie…
10153b7 
…ws (#110772) (#111322)

* [APM] Fix loading message missing or inconsistent in various list views

* fix types and i18n

* fix comment

* PR review comments

* fix JVM loading message

Co-authored-by: Miriam <31922082+MiriamAparicio@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smith smith smith approved these changes

@sorenlouv sorenlouv Awaiting requested review from sorenlouv

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:fix Team:APM - DEPRECATED Use Team:obs-ux-infra_services. v7.16.0
Projects
None yet
Milestone
No milestone
4 participants
@MiriamAparicio @elasticmachine @kibanamachine @smith