Skip to content

[APM] Add section titles to span detail modal#20717

Merged
formgeist merged 8 commits intoelastic:masterfrom
formgeist:apm-span-details-add-titles
Jul 13, 2018
Merged

[APM] Add section titles to span detail modal#20717
formgeist merged 8 commits intoelastic:masterfrom
formgeist:apm-span-details-add-titles

Conversation

@formgeist
Copy link
Contributor

@formgeist formgeist commented Jul 12, 2018

Fixes #18184

Adding titles for DB statement and stacktrace on the Span details modal.

screen shot 2018-07-12 at 14 17 31

@formgeist formgeist added v7.0.0 Team:APM - DEPRECATED Use Team:obs-ux-infra_services. enhancement New value added to drive a business result labels Jul 12, 2018
@formgeist formgeist self-assigned this Jul 12, 2018
@formgeist formgeist requested a review from sorenlouv July 12, 2018 12:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui

font-size: ${fontSize};
color: ${colors.gray1};
`;

Copy link
Member

Choose a reason for hiding this comment

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

Beautiful!
We already have a couple of headers. Maybe we could go over them at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sqren Made sense to re-use those, although added a new smaller size to match the other style I created.

export const HeaderXSmall = styled.h4`
margin: ${px(units.plus)} 0;
font-size: ${fontSize};
${props => props.css};
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uhhh, that's very nice 👍

Copy link
Member

Choose a reason for hiding this comment

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

yeah, it let's us have some standard headers, that can still be modified locally.

Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

🎉

@elasticmachine
Copy link
Contributor

💔 Build Failed

@formgeist
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@formgeist
Copy link
Contributor Author

@sqren do you think I need to update the tests first?

@sorenlouv
Copy link
Member

sorenlouv commented Jul 12, 2018

Ah, yes. Snapshots will need updating. From x-pack folder run this:

node scripts/jest apm --updateSnapshot

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@formgeist formgeist merged commit ecefab5 into elastic:master Jul 13, 2018
formgeist added a commit that referenced this pull request Jul 13, 2018
* Adding fontSize from variables

* SectionHeader style added

* Adding section headers

Needed titling for DB statement and Stacktrace on the page

* Pluralization

* Adding fontSize variable

* Creating new header title style

* Moving title into Stacktrace container

* Updated snapshots
@formgeist formgeist deleted the apm-span-details-add-titles branch July 13, 2018 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New value added to drive a business result Team:APM - DEPRECATED Use Team:obs-ux-infra_services. v7.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants