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

UI Polish for Session viewer frame + buttons #128188

Merged
merged 16 commits into from
Mar 29, 2022

Conversation

animehart
Copy link
Contributor

@animehart animehart commented Mar 21, 2022

Summary

UI Polish/Fixes based on QA Notes

Detail panel

➡️ Link to Figma file

  • Detail Panels sizing: Min width: 320 px; Max width: 640 px; Default width:w 360 px;
  • Fix typography in Detail Items
  • Detail Items height: 32 px
  • Fix spacing between Items
  • Fix padding between last Item and next section. Should be 16 px
  • Add Border-left to Details Panel Border-left box-shadow: 1px 0px 0px 0px #D3DAE6 inset;
  • Fix button states for Detail Panel ➡️ Toggle button specs in EUI

Toolbar

Find results interaction

  • Highlighted findings specs: Selected state: background: #FEC514;, Default state: background: #FEC51452;
  • No border-radius for highlight
  • Jump to findings below the viewport should scroll the session viewer content
  • There is an issue with a long text in the search field. It overlaps with stepper-control. Text should go under stepper control. Example EuiFieldSearch https://codesandbox.io/s/hz7boq?file=/demo.js

Session viewer frame

@animehart animehart linked an issue Mar 21, 2022 that may be closed by this pull request
21 tasks
@animehart
Copy link
Contributor Author

Screen Shot 2022-03-21 at 4 03 50 PM

Still WIP:

  • Details Panel
  • Find results interaction ??

@mitodrummer
Copy link
Contributor

If possible can you link the QA notes.

@mitodrummer
Copy link
Contributor

Screen Shot 2022-03-21 at 4 03 50 PM

Still WIP:

  • Details Panel
  • Find results interaction ??

looks like a rogue ';' got into your tsx somewhere.

Copy link
Contributor

@mitodrummer mitodrummer left a comment

Choose a reason for hiding this comment

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

Looking great, just a few notes.

fetchNextPage={fetchNextPage}
fetchPreviousPage={fetchPreviousPage}
setSearchResults={setSearchResults}
timeStampOn={displayOptions.timestamp}
Copy link
Contributor

Choose a reason for hiding this comment

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

note: think on the future we can replace to pass only one property displayOptions={displayOptions} to suit all display options (don't need to do it right now)

@animehart
Copy link
Contributor Author

Screen Shot 2022-03-22 at 4 10 38 PM

Still WIP:

  • Highlight color state for searched object (32% opacity color.warning default, color.warning when chosen)
  • There's a bug where the Timestamp moved to the left when user search some text

@animehart
Copy link
Contributor Author

Screen Shot 2022-03-23 at 12 59 44 PM

@mitodrummer
Copy link
Contributor

Screen Shot 2022-03-22 at 4 10 38 PM

Still WIP:

  • Highlight color state for searched object (32% opacity color.warning default, color.warning when chosen)
  • There's a bug where the Timestamp moved to the left when user search some text

Yea, I noticed the timestamp is wrapped in a weird way when the command is very long. float: right might not be the best approach. perhaps it should be put inline with the rest of the command line and use flexbox to align it to the right, or have a spacer to push it to the right.

@animehart animehart self-assigned this Mar 24, 2022
@animehart animehart added the Team: AWP: Platform Adaptive Workload Protection Platform team from Security Solution label Mar 24, 2022
@animehart animehart added release_note:enhancement v8.2.0 backport:skip This commit does not require backporting labels Mar 24, 2022
Copy link
Contributor

@opauloh opauloh left a comment

Choose a reason for hiding this comment

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

great, only a few notes

Copy link
Contributor

@mitodrummer mitodrummer left a comment

Choose a reason for hiding this comment

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

just the hardcoded font-size, other than that looks good.

Copy link
Contributor

@opauloh opauloh left a comment

Choose a reason for hiding this comment

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

lgtm!

@animehart animehart requested a review from a team March 29, 2022 13:38
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
sessionView 94 95 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
sessionView 50.3KB 51.5KB +1.2KB

History

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

cc @animehart

@animehart animehart merged commit 3bd3eb1 into elastic:main Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:enhancement Team: AWP: Platform Adaptive Workload Protection Platform team from Security Solution v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Security Solution] Session view UI polish based on the QA notes
5 participants