Skip to content

[RUM Dashboard] Rum usability improvement#76024

Merged
shahzad31 merged 10 commits intoelastic:masterfrom
shahzad31:rum-usability-improvement
Sep 4, 2020
Merged

[RUM Dashboard] Rum usability improvement#76024
shahzad31 merged 10 commits intoelastic:masterfrom
shahzad31:rum-usability-improvement

Conversation

@shahzad31
Copy link
Copy Markdown
Contributor

@shahzad31 shahzad31 commented Aug 26, 2020

Summary

Fixes #74835

Used a select component for breakdown selector

image

@shahzad31 shahzad31 added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 labels Aug 27, 2020
@shahzad31 shahzad31 self-assigned this Aug 27, 2020
@shahzad31 shahzad31 marked this pull request as ready for review August 27, 2020 06:51
@shahzad31 shahzad31 requested a review from a team August 27, 2020 06:51
Copy link
Copy Markdown
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.

Left some comments. The i18n definitely needs added, but use your discretion for the rest of my comments.

const items: BreakdownItem[] = [
{
name: 'Browser',
name: '- No breakdown -',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All of these name properties should have i18n translations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done


const onBreakdownChange = (values: BreakdownItem[]) => {
setBreakdowns(values);
const onBreakdownChange = (value: BreakdownItem | null) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you need to define this function? Couldn't you just pass onBreakdownChange={setBreakdown} into the BreakdownFilter?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

breakdown: {
terms: {
field: breakdownItem.fieldName,
size: 9,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why 9?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was mentioned in the issue that we need to show top 9 by volume #69712

const items: BreakdownItem[] = [
{
name: 'Browser',
name: '- No breakdown -',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think the - adds anything. No breakdown (with i18n) would be fine.

@botelastic botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Aug 27, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@shahzad31 shahzad31 requested a review from smith September 1, 2020 16:22
@shahzad31
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@shahzad31
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@shahzad31
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
apm 1235 -1 1236

async chunks size

id value diff baseline
apm 4.9MB -4.2KB 4.9MB

History

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

@shahzad31 shahzad31 merged commit 33d366a into elastic:master Sep 4, 2020
@shahzad31 shahzad31 deleted the rum-usability-improvement branch September 4, 2020 12:05
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Sep 4, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
shahzad31 added a commit that referenced this pull request Sep 4, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@sorenlouv sorenlouv removed the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RUM dashboard] usability improvements

5 participants