[Security Solution][THI] replace deprecated styled-components within detections folder#210942
[Security Solution][THI] replace deprecated styled-components within detections folder#210942PhilippeOberti wants to merge 1 commit intoelastic:mainfrom
Conversation
PhilippeOberti
added
release_note:skip
PhilippeOberti
force-pushed
the
styled-components-3
branch
from
5a6169a to
80cb2ad
Compare
PhilippeOberti
force-pushed
the
styled-components-3
branch
from
80cb2ad to
8f7150f
Compare
…detections folder
PhilippeOberti
force-pushed
the
styled-components-3
branch
from
8f7150f to
82de12b
Compare
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
|
Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations) |
| <DataStatsWrapper> | ||
| <div | ||
| css={css` | ||
| width: 250px; |
There was a problem hiding this comment.
save this as const as well?
| display: flex; | ||
| flex-direction: column; | ||
| position: relative; | ||
| overflow: hidden; |
There was a problem hiding this comment.
I noticed the scrolling in treemap is gone, we should change it to auto / bring back the prop
There was a problem hiding this comment.
I don't see any height changes in the code, but for some reason there is now blank space under the charts. The scroll bar shouldn't appear when all three are on the same row 🤔
maximpn
left a comment
maximpn
left a comment
There was a problem hiding this comment.
@PhilippeOberti Thanks for replacing deprecated styled-components 🙏
Rules Management changes are trivial and testing locally didn't reveal any issues. However, styles changes have been proofed not working in production. Could you resolved conflicts and deploy the PR so we and the other teams are able to test the changes?
| filters={alertMergedFilters} | ||
| signalIndexName={signalIndexName} | ||
| defaultStackByOption={defaultRuleStackByOption} | ||
| title={i18n.HISTOGRAM_HEADER} |
There was a problem hiding this comment.
Is there a reason for adding a title?
There was a problem hiding this comment.
I'm not really adding the title, I just moved it from here
With the AI for SOC effort I kinda dropped this a bit. I might be a few weeks before I get back to this. I will then rebase, fix all the conflict then deploy the PR for testing and will ping everyone again 👍 |
|
|
||
| import { i18n } from '@kbn/i18n'; | ||
|
|
||
| export const COUNT_TABLE_COLUMN_TITLE = i18n.translate( |
There was a problem hiding this comment.
It'd be nice to go through file by file and just remove dead code 😞
| filters, | ||
| }} | ||
| getLensAttributes={getLensAttributes} | ||
| height={chartHeight ?? CHART_HEIGHT} |
|
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
|
Closing as the code has changed too much since last worked on, and this work isn't a high priority right now... |
6 participants
Summary
This PR is a follow up of this one and that one, aiming at replacing the usages
styled-componentswith@emotion/reactor@emotion/styledin thesecurity_solution/public/detectionsfolder.Similar to the previous PRs, along side the replacement of the deprecated
styled-components, this PR performs some cleanup of unused props/translations/...Absolutely no UI or behavior logic changes should be introduced by this PR.
There are more changes coming, but having all of these in a single PR would have been too much to review. I decided to keep the changes focused to a single area as much as possible, to facilitate reviewing that the UI has not changed.
You will notice different approaches while removing the tokens:
css from '@emotions/react'as the components using the tokens were already usingeuiThemeor adding it was necessary or straightforward and required the minimal amount of changesstyled-componentsto@emotion/styledwas enough and required no further code changesSome screenshots:
Checklist