-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[SIEM] Table columns, number related design tweaks #48969
[SIEM] Table columns, number related design tweaks #48969
Conversation
Pinging @elastic/siem (Team:SIEM) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this @stephmilovic. It's looking good. Left two small comments. Once updated, I can give it a LGTM :)
x-pack/legacy/plugins/siem/public/components/page/hosts/authentications_table/index.tsx
Outdated
Show resolved
Hide resolved
💚 Build Succeeded |
💚 Build Succeeded |
Pushed a change to do relative time in past hour @MichaelMarcialis. Is there anywhere else besides the Auth table that you want to see this? |
💚 Build Succeeded |
Thanks, @stephmilovic. If we're talking about just the hosts and host details pages, I'd say we'd want that same relative/absolute time behavior on:
If we're talking globally across SIEM, we'll probably want it to also behave this way on:
Also, if we are indeed talking globally across SIEM, I imagine we'll want the following number-based table columns to also be right-aligned to match:
|
@MichaelMarcialis all your requested changes have been made. Some more screenshots: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks great to me. Thanks, @stephmilovic!
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
x-pack/legacy/plugins/siem/public/components/formatted_date/index.test.tsx
Outdated
Show resolved
Hide resolved
* - the raw date value (e.g. 2019-03-22T00:47:46Z) | ||
*/ | ||
|
||
export const FormattedRelativePreferenceDate = ({ value }: { value?: string | number | null }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ToFormattedRelativePreferenceDate
or to RelativeFormattedPreferenceDate
? That is the question!
Seems there's some dispute around if the Age modifier should be before or after Shape. Good to know us engineers aren't the only ones that care deeply about specificity... 😅
(I just learned this recently, so just a shameless plug for that wiki article -- no change necessary!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite
x-pack/legacy/plugins/siem/public/components/formatted_date/index.tsx
Outdated
Show resolved
Hide resolved
x-pack/legacy/plugins/siem/public/components/ml/tables/get_anomalies_host_table_columns.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all these awesome fixes @stephmilovic! Checked out, tested locally, and performed a code review. Added a few infotainment comments, but no changes necessary. LGTM! 👍
💚 Build Succeeded |
Summary
I realized I still had a couple issues open from the Product Design Meta I opened in April, so thought I'd get those small changes in.
Resolves https://github.com/elastic/siem-team/issues/253
Resolves https://github.com/elastic/siem-team/issues/254
I also set the width on a few of these numeral value in some of the more crowded tables, I think it looks a bit better but I want to see what @MichaelMarcialis thinks.
Auth Table

Before:
After:

Uncommon Processes Table

Before:
After:

DNS Table

Before:
After:

Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.This was checked for cross-browser compatibility, including a check against IE11Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsUnit or functional tests were updated or added to match the most common scenariosThis was checked for keyboard-only and screenreader accessibilityFor maintainers
This was checked for breaking API changes and was labeled appropriatelyThis includes a feature addition or change that requires a release note and was labeled appropriately