styling updates to alerts table and alignment of elements #108507
Conversation
katrin-freihofner
added
release_note:skip
|
@mgiota I think this PR is a prerequisite for the scrolling issue. #108188 Current status One thing I need help with here is the alignment of the inspect icon. @mdefazio commented on an example of what this should look like. cc @elastic/logs-ui |
|
@katrin-freihofner Maybe you would like to sync up with @afgomez on that, since it seems he has already an open PR for aligning the buttons on the left #108215. He has a note to fix some alignment issues in another PR |
|
@elasticmachine merge upstream |
|
merge conflict between base and head |
| @@ -345,7 +336,7 @@ const TGridStandaloneComponent: React.FC<TGridStandaloneProps> = ({ | |||
|
|
|||
| return ( | |||
| <InspectButtonContainer> | |||
| <StyledEuiPanel data-test-subj="events-viewer-panel" $isFullScreen={false}> | |||
| <AlertsTableWrapper data-test-subj="events-viewer-panel"> | |||
There was a problem hiding this comment.
to get the failing test to pass, you just need this data-test-subj to appear on the wrapper or rendered on a separate element in this component
There was a problem hiding this comment.
Thanks 👍 I was helping Katrin get these last failing CI steps to pass. For my own sanity could you explain why I had to bump this to the outer wrapper? (Which is passing).
With the original structure the element is still there, in the DOM, but doesn't get returned?
There was a problem hiding this comment.
From the screenshot, no I cannot, that looks like it should pass 😢
There was a problem hiding this comment.
Super weird! But thanks for looking.
| padding-top: 0; | ||
| padding-bottom: 0; | ||
| `} | ||
| const AlertsTableWrapper = styled.div` |
There was a problem hiding this comment.
also might be worth noting that tGrid runs in 2 "modes": integrated, where the redux actions/state are used within a bigger app like security solution, or in standalone, where all of those things are run in a self contained way. Tldr if you want these styling changes to be reflected in security solution as well, need to make these same changes https://github.com/elastic/kibana/blob/master/x-pack/plugins/timelines/public/components/t_grid/integrated/index.tsx#L68
There was a problem hiding this comment.
The integrated file was updated, but these look to be setup slightly differently: #108903
There was a problem hiding this comment.
in my humble opinion, I feel like it is better to remove the the EuiPanel and just use a div than disabling everything from this component.
💚 Build SucceededMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
…109049) Co-authored-by: Kerry Gallagher <[email protected]> Co-authored-by: Kerry Gallagher <[email protected]>
Summary
Summarize your PR. If it involves visual changes include a screenshot or gif.
Fixes https://github.com/elastic/observability-design/issues/83
Checklist
Delete any items that are not applicable to this PR.
For maintainers