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

[Uptime]Fix/issue 40584 section headline should be inside panel #43468

Merged
merged 13 commits into from
Aug 30, 2019
Merged

[Uptime]Fix/issue 40584 section headline should be inside panel #43468

merged 13 commits into from
Aug 30, 2019

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Aug 16, 2019 •
edited
Loading

Summary

Section headline should be inside panel, In all sections in Uptime, title has been moved inside to reflect new design approach.

Resolves #40584

In Uptime App Section Titles were being displayed outside container like this
image

In this PR i have made changes so that Section Title appears inside container to have better relativity. For example Monitor Status
image

@elasticmachine
Copy link
Contributor

💔 Build Failed

@shahzad31 shahzad31 added Feature:Uptime Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability and removed Feature:Uptime labels Aug 16, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

justinkambic
justinkambic reviewed Aug 23, 2019
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

For me, viewing this locally in latest Chrome, I am seeing incongruence in the headlines between these two panels on the Overview page:
image

There's also some weirdness going on on the Monitor page with the legend and the badge.

Untitled 2

cc @andrewvc regarding the badge on the Monitor Page's ping list, hadn't we discussed removing it in previously?

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@shahzad31
Copy link
Contributor Author

jenkins, retest this please

@elasticmachine
Copy link
Contributor

💔 Build Failed

@spalger
Copy link
Contributor

spalger commented Aug 28, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@justinkambic
Copy link
Contributor

This is looking good. I don't see the weirdness anymore. But now that badge with the ping count is all by itself and it looks lonely and out of place.

WDYT about moving it next to the header in the panel?

cc @andrewvc @katrin-freihofner

image

@andrewvc
Copy link
Contributor

The ping count isn't that useful, and the design we're moving toward is in the mock below. I think we should just remove it.

image

@katrin-freihofner
Copy link
Contributor

WDYT about moving it next to the header in the panel?

Removing it is fine. If we want to keep it I would suggest to add it to the headline 67 Pings over time. Btw the padding of both panels, pings and monitor duration seems off. Would you mind adding more (8px) whitespace on the bottom of the panel (paddingSize='s')?

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@shahzad31
Copy link
Contributor Author

I agree, going to remove ping count.

@justinkambic
Copy link
Contributor

It's somewhat meaningless in production when the number gets large.

++ to this and what's come out of the other comments.

@nik9000
Copy link
Member

nik9000 commented Aug 29, 2019

@elasticmachine, run elasticsearch-ci/docs

@nik9000
Copy link
Member

nik9000 commented Aug 29, 2019

@elasticmachine, run elasticsearch-ci/docs

@elasticmachine
Copy link
Contributor

💔 Build Failed

@shahzad31
Copy link
Contributor Author

Have improved paddings for charts
image
@katrin-freihofner

@shahzad31
Copy link
Contributor Author

shahzad31 commented Aug 30, 2019 •
edited
Loading

Have added loader for charts

Screen Capture on 2019-08-30 at 10-26-00

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

justinkambic
justinkambic approved these changes Aug 30, 2019
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

This is a great change/enhancement. I just smoke tested locally and it works great.

LGTM

...ck/legacy/plugins/uptime/public/components/functional/charts/chart_wrapper/chart_wrapper.tsx
loading?: boolean;
}

export const ChartWrapper: FC<Props> = ({ loading = false, height = '100%', children }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice addition 👍

@shahzad31 shahzad31 merged commit 7fd09c9 into elastic:master Aug 30, 2019
@shahzad31 shahzad31 deleted the fix/issue-40584--section-headline-should-be-inside-panel branch August 30, 2019 22:38
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 2, 2019
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into chore/console…
a6b8750 
…-to-np-ready

* 'master' of github.com:elastic/kibana: (409 commits)
  [ML] Data frame analytics: Fix source index checks. (elastic#44479)
  [Code] try fix this test (elastic#43692)
  skip flaky suite (elastic#44572) (elastic#42111) (elastic#44286) (elastic#43557) (elastic#42567)
  skip flaky suite (elastic#44560)
  skip flaky suite (elastic#44250)
  disable flaky suite (elastic#41336)
  [code] Update download URLs for go lsp. (elastic#44581)
  disable flaky suite (elastic#44575)
  disable flaky suite (elastic#44576)
  [Code] add functional test to verify lang server full api symbol/reference counts (elastic#44051)
  Improve Storybook scripts and load time (elastic#44511)
  Update Dependencies (elastic#44519)
  Remove use of injecti18n in Embeddables plugin (elastic#44043)
  [SIEM] Adds a configuraton option for the default SIEM date time range (elastic#44540)
  [Uptime]Fix/issue 40584  section headline should be inside panel (elastic#43468)
  disable flaky suite (elastic#22322)
  Changes network to use ECS fields (elastic#44392)
  Fix 'workpad flash' when loading new workpad (elastic#44387)
  [renovate] bump new PR version labels
  Update dependency cmd-shim to ^2.1.0 (elastic#44034)
  ...

# Conflicts:
#	src/legacy/core_plugins/console/public/index.html
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Sep 5, 2019
@shahzad31
[Uptime]Fix/issue 40584 section headline should be inside panel (elas…
075fb6b 
…tic#43468)

* move title inside panel

* fix monitor list title

* update title in each panel and paddings

* update unit tests snapshots

* make section titles symmeteric

* update snapshots

* Add chart wrapper to improve UX experience and padding arounds charts

* removed ping list count

* removed unnecessary spacer

* update test snaps
This was referenced Sep 5, 2019
Merged
Merged
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Sep 5, 2019
@shahzad31
[Uptime]Fix/issue 40584 section headline should be inside panel (elas…
60df08d 
…tic#43468)

* move title inside panel

* fix monitor list title

* update title in each panel and paddings

* update unit tests snapshots

* make section titles symmeteric

* update snapshots

* Add chart wrapper to improve UX experience and padding arounds charts

* removed ping list count

* removed unnecessary spacer

* update test snaps
shahzad31 added a commit that referenced this pull request Sep 5, 2019
@shahzad31
[Uptime]Fix/issue 40584 section headline should be inside panel (#43468…
d7a3e1c 
…) (#44878)

* move title inside panel

* fix monitor list title

* update title in each panel and paddings

* update unit tests snapshots

* make section titles symmeteric

* update snapshots

* Add chart wrapper to improve UX experience and padding arounds charts

* removed ping list count

* removed unnecessary spacer

* update test snaps
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Sep 10, 2019
@shahzad31
[Uptime]Fix/issue 40584 section headline should be inside panel (elas…
9b57a4d 
…tic#43468)

* move title inside panel

* fix monitor list title

* update title in each panel and paddings

* update unit tests snapshots

* make section titles symmeteric

* update snapshots

* Add chart wrapper to improve UX experience and padding arounds charts

* removed ping list count

* removed unnecessary spacer

* update test snaps
shahzad31 added a commit that referenced this pull request Sep 10, 2019
@shahzad31
[Uptime]Fix/issue 40584 section headline should be inside panel (#43468…
e981b55 
…) (#44879)

* move title inside panel

* fix monitor list title

* update title in each panel and paddings

* update unit tests snapshots

* make section titles symmeteric

* update snapshots

* Add chart wrapper to improve UX experience and padding arounds charts

* removed ping list count

* removed unnecessary spacer

* update test snaps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinkambic justinkambic justinkambic approved these changes

Assignees

@shahzad31 shahzad31

Labels
release_note:fix Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.5.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Uptime Ui] section headlines should be within panel
7 participants
@shahzad31 @elasticmachine @spalger @justinkambic @andrewvc @katrin-freihofner @nik9000