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

[APM] Visual improvements for new APM layout with left navigation #101360

Merged
merged 5 commits into from
Jun 7, 2021

Conversation

sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Jun 4, 2021

Follow-up to: #101044
Closes: #101355

image

TODO

  • Adopt full page height of the content panel
  • Move Service tabs navigation into the PageHeader
  • Wrap the Service map in a EuiPanel
  • Service maps height doesn't account for the new header height
  • JVM detail page: Remove the horizontal rule below the detail header
  • Settings: Remove the light grey content background color
  • Agent configuration: Create view is not wrapped in the new page template, missing the navigation
  • Service inventory and Traces: Remove the panel padding around the tables
  • Truncation or wrapping missing in the PageHeader title (blocked) [APM] Solution navigation UI polish #101355

@sorenlouv sorenlouv requested review from a team as code owners June 4, 2021 08:54
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Jun 4, 2021
@elasticmachine
Copy link
Contributor

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

@sorenlouv sorenlouv added release_note:skip Skip the PR/issue when compiling release notes v7.14.0 labels Jun 4, 2021
@sorenlouv sorenlouv marked this pull request as draft June 4, 2021 08:56
@formgeist
Copy link
Contributor

formgeist commented Jun 4, 2021

@sqren added another item to the list; I found that the service map canvas extends beyond the viewport height so there's a weird scroll on the page. I suppose that's because the new header height isn't accounted for. There's a reference description in #101355

@sorenlouv
Copy link
Member Author

Thanks @formgeist!

@sorenlouv sorenlouv marked this pull request as ready for review June 4, 2021 13:22
@sorenlouv sorenlouv force-pushed the apm-minor-tweaks-to-new-layout branch from 5b95d8a to fea2b90 Compare June 4, 2021 13:23
@sorenlouv sorenlouv changed the title [APM] Minor tweaks [APM] Minor tweaks to be side nav Jun 4, 2021
@sorenlouv sorenlouv changed the title [APM] Minor tweaks to be side nav [APM] Design improvements for new APM layout with left navigation Jun 6, 2021
@sorenlouv sorenlouv force-pushed the apm-minor-tweaks-to-new-layout branch from 8a0e17f to 306e128 Compare June 6, 2021 21:26
@sorenlouv sorenlouv changed the title [APM] Design improvements for new APM layout with left navigation [APM] Visual improvements for new APM layout with left navigation Jun 7, 2021
Copy link
Contributor

@streamich streamich left a comment

Choose a reason for hiding this comment

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

AppServices codeowners change LGTM.

@sorenlouv sorenlouv enabled auto-merge (squash) June 7, 2021 11:29
Comment on lines +260 to 304
export function EditAgentConfigurationRouteView(props: RouteComponentProps) {
const { search } = props.history.location;

// typescript complains because `pageStop` does not exist in `APMQueryParams`
// Going forward we should move away from globally declared query params and this is a first step
// @ts-expect-error
const { name, environment, pageStep } = toQuery(search);

const res = useFetcher(
(callApmApi) => {
return callApmApi({
endpoint: 'GET /api/apm/settings/agent-configuration/view',
params: { query: { name, environment } },
});
},
[name, environment]
);

return (
<ApmMainTemplate pageTitle="Settings">
<SettingsTemplate {...props}>
<AgentConfigurationCreateEdit
pageStep={pageStep || 'choose-settings-step'}
existingConfigResult={res}
/>
</SettingsTemplate>
</ApmMainTemplate>
);
}

export function CreateAgentConfigurationRouteView(props: RouteComponentProps) {
const { search } = props.history.location;

// Ignoring here because we specifically DO NOT want to add the query params to the global route handler
// @ts-expect-error
const { pageStep } = toQuery(search);

return (
<ApmMainTemplate pageTitle="Settings">
<SettingsTemplate {...props}>
<AgentConfigurationCreateEdit
pageStep={pageStep || 'choose-service-step'}
/>
</SettingsTemplate>
</ApmMainTemplate>
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not new but was simply moved from another file

Copy link
Member

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

Really like how this utilizes space better! And I've noticed how this already massively helps in the perceived "walls" between the different Observability apps.

};
}) {
}: Props) {
const tabs = useTabs({ serviceName, selectedTab });
Copy link
Member

Choose a reason for hiding this comment

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

I like the use of hooks here 👍

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1600 1599 -1

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.3MB 4.3MB +102.0B

History

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

@sorenlouv sorenlouv merged commit 999faec into elastic:master Jun 7, 2021
@sorenlouv sorenlouv deleted the apm-minor-tweaks-to-new-layout branch June 7, 2021 15:08
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 7, 2021
* master: (90 commits)
  Fix UI breaks on providing long search keyword in 'Search Box' (elastic#101385)
  Adds css class to EuiDescriptionListDescription in order to break word on exception details card (elastic#101481)
  [Lens] Increase timings for drag and drop tests (elastic#101380)
  [Lens] Fix editor react error on configuration panel (elastic#101367)
  [Fleet] Move integrations to a separate app (elastic#99848)
  Fix incorrect message displayed on importing Timeline Templates (elastic#101288)
  [Cases] RBAC (elastic#95058)
  [APM] Visual improvements for new APM layout with left navigation (elastic#101360)
  [master] More precise alerts matching (elastic#99820)
  [Lens] Value in legend (elastic#101353)
  Revert "[Reporting] ILM policy for managing reporting indices (elastic#100130)" (elastic#101358)
  [Discover] Fix header row of data grid in Firefox (elastic#101374)
  Add link to advanced setting in Discover (elastic#101154)
  Url service locators (elastic#101045)
  [Timelion] Update the removal message to mention the exact version (elastic#100994)
  [Security Solution][Detection Engine] Test cases for alias failure test cases where we don't copy aliases correctly (elastic#101437)
  [Event Log] Adding `type_id` to saved object array in event log (elastic#100939)
  [Reporting] Add `location.url` info to console message logs (elastic#101427)
  [Security Solutions][Detection Engine] Fixes timestamp bugs within source indexes when the formats are not ISO8601 format (elastic#101349)
  Improve Task Manager instrumentation (elastic#99160)
  ...
@sorenlouv sorenlouv added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 9, 2021
@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 Jun 9, 2021
…01360) (#101747)

Co-authored-by: Søren Louv-Jansen <sorenlouv@gmail.com>
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:APM All issues that need APM UI Team support v7.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM] Solution navigation UI polish
6 participants