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

[Design Audit] Cleanup small layout issues on Case pages #118128

Merged
merged 9 commits into from
Nov 11, 2021
Merged

[Design Audit] Cleanup small layout issues on Case pages #118128

merged 9 commits into from
Nov 11, 2021

Conversation

hbharding
Copy link
Contributor

@hbharding hbharding commented Nov 10, 2021 •
edited
Loading

Summary

This PR addresses several small layout issues from a recent design audit.

Removed double padding from Case detail page layout

We were using some wrapper components that applied unnecessary padding.

Before After
image image

Fix double page header on Cases page in Observability

The actions now appear in the same row as the Page title, as intended.

Before After
image image

Improve layout of items in right area of page header

The items were spaced unevenly and were wrapping lines unnecessarily

Before After
image image

Checklist

Delete any items that are not applicable to this PR.

  • This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)

For maintainers

@hbharding hbharding added release_note:enhancement v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Nov 10, 2021
@hbharding hbharding requested a review from a team as a code owner November 10, 2021 01:22
@hbharding hbharding changed the title Obs rac layout cleanup [Design Audit] Cleanup small layout issues on Case pages Nov 10, 2021
claudiopro
claudiopro suggested changes Nov 10, 2021
View reviewed changes
Copy link
Contributor

@claudiopro claudiopro left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me! I love the before/after screenshot, it helps a lot to better understand the change 👍
Please take a look at the CI check failure, otherwise fine to me. You may have to wait for a code review from the code owner team elastic/security-threat-hunting.

x-pack/plugins/observability/public/components/app/cases/translations.ts Show resolved Hide resolved
@mgiota
Copy link
Contributor

mgiota commented Nov 10, 2021 •
edited
Loading

@hbharding Here are the labels you need to have in your PR:

  • auto-backport
  • v8.0.0
  • v8.1.0
  • release_note: skip (I see you added also release_note: enhancement, to be honest so far I always used the skip one, not sure when we should use enhancement)
  • Theme: rac
  • Team: Actionable Observability

And here's an example of a recent PR with the above mentioned labels #116446

@hbharding hbharding added v8.1.0 auto-backport Deprecated - use backport:version if exact versions are needed Theme: rac label obsolete Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" and removed release_note:enhancement labels Nov 10, 2021
@hbharding
Copy link
Contributor Author

The related issue for this in a private repo. Apologies, i'll make it in kibana repo next time :)

@hbharding hbharding requested a review from claudiopro November 10, 2021 21:33
@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
cases 324.7KB 323.4KB -1.3KB
observability 379.3KB 379.2KB -145.0B
total -1.4KB

History

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

claudiopro
claudiopro approved these changes Nov 10, 2021
View reviewed changes
Copy link
Contributor

@claudiopro claudiopro left a comment

Choose a reason for hiding this comment

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

LGTM! Ship it :shipit:

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-cases (Team:Threat Hunting:Cases)

semd
semd approved these changes Nov 11, 2021
View reviewed changes
Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

Thank you @hbharding !
tested locally LGTM 🚀

@hbharding hbharding merged commit da7f7b2 into elastic:main Nov 11, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 11, 2021
@kibanamachine
[Design Audit] Cleanup small layout issues on Case pages (elastic#118128
bd53715 
)

* fixed line wrapping

* Prevent title from breaking word on wrap.

* remove extra padding

* Fixes "Cases" double header. Actions on same row.

* remove leftover code / fix check

* fix i18n check
@kibanamachine kibanamachine mentioned this pull request Nov 11, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 8.0

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Nov 11, 2021
@kibanamachine
[Design Audit] Cleanup small layout issues on Case pages (#118128) (#…
1082599 
…118360)

* fixed line wrapping

* Prevent title from breaking word on wrap.

* remove extra padding

* Fixes "Cases" double header. Actions on same row.

* remove leftover code / fix check

* fix i18n check

Co-authored-by: Henry Harding <[email protected]>
dmlemeshko pushed a commit that referenced this pull request Nov 29, 2021
@dmlemeshko
[Design Audit] Cleanup small layout issues on Case pages (#118128)
fb72b6a 
* fixed line wrapping

* Prevent title from breaking word on wrap.

* remove extra padding

* Fixes "Cases" double header. Actions on same row.

* remove leftover code / fix check

* fix i18n check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgiota mgiota mgiota left review comments

@semd semd semd approved these changes

@claudiopro claudiopro claudiopro approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" Theme: rac label obsolete v8.0.0 v8.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@hbharding @mgiota @kibanamachine @elasticmachine @claudiopro @semd @cnasikas