[Cloud Posture] search bar styles update #128361
Conversation
|
|
||
| export const FINDINGS_SEARCH_PLACEHOLDER = i18n.translate( | ||
| 'xpack.csp.findings.searchBar.searchPlaceholder', | ||
| { defaultMessage: 'Search findings (eg. resource.type : "API Server")' } |
There was a problem hiding this comment.
The example is falsely,
To filter API Server the user would need to apply the following rule.section: "API Server"
orouz
added
release_note:skip
orouz
force-pushed
the
csp_findings_search
branch
from
1825a6e to
53b31b6
Compare
orouz
force-pushed
the
csp_findings_search
branch
from
53b31b6 to
f0950b2
Compare
| <Route path="*" component={UnknownRoute} /> | ||
| </Switch> | ||
| </I18nProvider> | ||
| <CspThemeProvider> |
There was a problem hiding this comment.
No need to wrap in a theme provider, our entire app is wrapped in a <KibanaThemeProvider /> which is a wrapper of <EuiThemeProvider />
There was a problem hiding this comment.
KibanaThemeProvider doesn't include the context for eui's emotion theme, which is what this bit adds
although if we manage to only use useEuiTheme() , we won't need this at all
removed this, not used anymore.
| import { useTheme as useEmotionTheme } from '@emotion/react'; | ||
| import { EuiTheme } from '../../../../../../src/plugins/kibana_react/common'; | ||
|
|
||
| export function useTheme() { |
There was a problem hiding this comment.
Lets drop this and use useEuiTheme from eui instead
There was a problem hiding this comment.
seems like useEuiTheme doesn't include the value of the emotion theme for euiPageBackgroundColor, and no similar value either. (will use that)
There was a problem hiding this comment.
It exists, look at shades in the colors documentation
colors.emptyShade
Used as the background color of primary page content and panels including modals and flyouts.
colors.body
The background color for the whole window (body) and is a computed value of colors.lightestShade. Provides denominator (background) value for contrast calculations.
| @@ -22,6 +22,13 @@ import type { DataView } from '../../../../../../src/plugins/data/common'; | |||
|
|
|||
| jest.mock('../../common/api/use_kubebeat_data_view'); | |||
| jest.mock('../../common/api/use_cis_kubernetes_integration'); | |||
| jest.mock('../../common/hooks/use_theme', () => ({ | |||
There was a problem hiding this comment.
Instead of mocking this I suggest adding a theme provider to our <TestProvider />
There was a problem hiding this comment.
we could this if you prefer, but we don't own the provider, we get it from eui for all kibana (not in our app providers) so adding it seems more complicated than mocking it.
There was a problem hiding this comment.
Not sure what you mean, we do add it explicitly, Kibana does not do that for us (we add it here)
| const pageHeader: EuiPageHeaderProps = { | ||
| pageTitle: FINDINGS, | ||
| }; | ||
| const FindingsPageTemplate = styled(CspPageTemplate)` |
There was a problem hiding this comment.
This doesn't seem to do anything, when I remove it and use CspPageTemplate directly the page looks the same. I think that removing the pageHeader prop removes the header, so no need for this hack
| <FindingsTable setQuery={setUrlQuery} {...findingsQuery} {...findingsResult} /> | ||
| <div | ||
| css={css` | ||
| padding: 24px; |
There was a problem hiding this comment.
We need to start using sizes from the eui theme, lets use size.l instead of a fixed value here
| const PageTitle = () => ( | ||
| <EuiTitle size="l"> | ||
| <h2> | ||
| <FormattedMessage id="xpack.csp.findings.findingsLabel" defaultMessage="Findings" /> |
There was a problem hiding this comment.
I think the ID should end with findingsTitle as this is a title
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
Summary
this PR includes changes to the CSP Findings SearchBar:
it also includes some code to add support for theme context (using emotion) to get access to
euiinternal vars, which were needed to satisfy the design.