-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Fix Timeline footer styling #59587
[SIEM] Fix Timeline footer styling #59587
Conversation
…line-footer-styles # Conflicts: # x-pack/legacy/plugins/siem/public/components/flyout/pane/index.tsx
Pinging @elastic/siem (Team:SIEM) |
x-pack/legacy/plugins/siem/public/components/timeline/footer/index.tsx
Outdated
Show resolved
Hide resolved
<TimelineKqlFetch id={id} indexPattern={indexPattern} inputId="timeline" /> | ||
{combinedQueries != 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.
I'm seeing a change in behavior while desk testing (not necessarily related to this location) where the Timeline is populated by default, even on a fresh reload of the SIEM app.
As a result, Create new timeline
appears to not work, because the timeline is populated with data even though there are no filters or KQL entered.
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.
Seeing the following in the JS console while desk testing (not necessarily related to this location) when clicking the Load more
button:
react_devtools_backend.js:6 Warning: Encountered two children with the same key, `j2xwvXABOOUskGlPa61E`. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.
in div (created by EventsTbody)
in EventsTbody (created by EventsComponent)
in EventsComponent
in div (created by EventsTable)
in EventsTable
in div (created by TimelineBody)
in TimelineBody
in Unknown
in Unknown
in Unknown (created by ConnectFunction)
in ConnectFunction (created by Query)
in div (created by EuiFlyoutBody)
in div (created by EuiFlyoutBody)
in div (created by EuiFlyoutBody)
in EuiFlyoutBody (created by StyledEuiFlyoutBody)
in StyledEuiFlyoutBody (created by Query)
in ManageTimelineContextComponent (created by Query)
in Query (created by TimelineQueryComponent)
in TimelineQueryComponent (created by EnhancedType)
in EnhancedType (created by ConnectFunction)
in ConnectFunction (created by TimelineComponent)
in div (created by TimelineContainer)
in TimelineContainer (created by TimelineComponent)
in TimelineComponent (created by Query)
in Query
in Unknown
in Unknown
in Unknown (created by ConnectFunction)
in ConnectFunction
in div (created by Resizable)
in Resizable (created by StyledResizable)
in StyledResizable (created by FlyoutPaneComponent)
in div (created by EuiFlyout)
in div (created by FocusLock)
in FocusLock (created by EuiFocusTrap)
in div (created by OutsideEventDetector)
in OutsideEventDetector (created by EuiFocusTrap)
in EuiOutsideClickDetector (created by EuiFocusTrap)
in EuiFocusTrap (created by EuiFlyout)
in EuiFlyout (created by FlyoutPaneComponent)
in div (created by EuiFlyoutContainer)
in EuiFlyoutContainer (created by FlyoutPaneComponent)
in FlyoutPaneComponent
in div (created by Visible)
in Visible
in Unknown (created by ConnectFunction)
in ConnectFunction (created by Query)
in Provider (created by App)
in App (created by ErrorBoundary)
in ErrorBoundary (created by DragDropContext)
in DragDropContext
in Unknown
in Unknown (created by ConnectFunction)
in ConnectFunction (created by Query)
in Query
in Unknown (created by HomePage)
in main (created by Main)
in Main (created by HomePage)
in div (created by WrappedByAutoSizer)
in WrappedByAutoSizer (created by HomePage)
in HomePage (created by PageRouterComponent)
in Route (created by PageRouterComponent)
in Switch (created by PageRouterComponent)
in Router (created by PageRouterComponent)
in ManageRoutesSpyComponent (created by PageRouterComponent)
in PageRouterComponent (created by AppPluginRootComponent)
in Unknown (created by AppPluginRootComponent)
in ThemeProvider (created by AppPluginRootComponent)
in ApolloProvider (created by AppPluginRootComponent)
in Provider (created by AppPluginRootComponent)
in ManageGlobalToaster (created by AppPluginRootComponent)
in AppPluginRootComponent (created by StartAppComponent)
in EuiContext (created by I18nContext)
in PseudoLocaleWrapper (created by I18nProvider)
in IntlProvider (created by I18nProvider)
in I18nProvider (created by I18nContext)
in I18nContext (created by StartAppComponent)
in EuiErrorBoundary (created by StartAppComponent)
in StartAppComponent (created by SiemAppComponent)
in Provider (created by SiemAppComponent)
in SiemAppComponent
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.
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.
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.
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.
.find('[data-test-subj="timeline-title"]') | ||
.first() | ||
.props().placeholder | ||
).toContain('Untitled Timeline'); |
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.
Is there an equivalent version of this test?
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.
x-pack/legacy/plugins/siem/public/components/timeline/body/column_headers/column_header.tsx
Outdated
Show resolved
Hide resolved
const handleResizeStop: ResizeCallback = (e, direction, ref, delta) => { | ||
onColumnResized({ columnId: header.id, delta: delta.width }); | ||
}; | ||
const resizableEnable = useMemo(() => ({ right: true }), []); |
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 pure function doesn't appear to capture any variables via closure, (as opposed to for example, resizableSize
below it).
Consider declaring resizableEnable
outside the scope of this component, because it doesn't appear that every instance of the component needs its own memoized version of resizableEnable
.
Seeing the following error when clicking the
|
x-pack/legacy/plugins/siem/public/components/timeline/properties/index.test.tsx
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
|
||
export const useThrottledResizeObserver = (wait = 100) => { | ||
const [size, setSize] = useState<{ width: number; height: number }>({ width: 0, height: 0 }); | ||
const onResize = useMemo(() => throttle(wait, setSize), [wait]); |
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.
is it possible to use useCallback
here?
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 @patrykkopycinski for all the hard work that went into the fixes and refactoring in this PR! 🙏
Cross-browser desk testing in Chrome 80.0.3987.132
, Firefox 74.0
, and Safari 13.0.5
is looking solid 👍👍👍
LGTM 🚀
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* master: (30 commits) [TSVB] fix text color when using custom background color (elastic#60261) Fix import to timefilter from in TSVB (elastic#60296) [NP] Get rid of usage redirectWhenMissing service (elastic#59777) [SIEM] Fix Timeline footer styling (elastic#59587) [ML] Fixes to error handling for analytics jobs and file data viz (elastic#60249) Give better stack traces for Unhandled Promise Rejection warnings (elastic#60235) resolves elastic#58905 (elastic#60120) Added variables button for text fields in Pagerduty component. (elastic#60189) adds test that action vars are rendered for alert action parms (elastic#60310) Closes 59786 by removing the update toast (elastic#60172) [EPM] Packages list tabs (elastic#60167) Added message variables button for Webhook body form field (elastic#60174) Revert "adds new test (elastic#60064)" [Maps] move MapSavedObject type out of telemetry (elastic#60127) [Reporting] Fix error handling for job handler in route (elastic#60161) [Endpoint] TEST: verify alerts page header says 'Alerts' (elastic#60206) EMT-248: implement ack resource to accept event payload to acknowledge agent actions (elastic#60218) Migrate dual validated range (elastic#59689) Embeddable triggers (elastic#58440) [Endpoint] Sample data generator CLI script (elastic#59952) ...
Summary
Use
EuiFlyoutFooter
component in place of custom javascriptuseResizeObserver
logicChecklist
Delete any items that are not applicable to this PR.
For maintainers