Skip to content

Commit

Permalink
Replace inline styles with css prop in src/ folder (#201563)
Browse files Browse the repository at this point in the history
## Summary

Part of the resolution of this issue: 
- #149246

Removes the `style` prop in React components and elements to avoid using
inline styles. Instead, it uses now the `emotion.css` prop to
dynamically attach all styles to the native `class` attribute.

### Motivation

Using inline styles at scale causes a performance penalty at rendering
time. It's way more efficient to attach styles to a single or several
classnames instead.

### How to review

From [Emotion's official
docs](https://emotion.sh/docs/css-prop#style-precedence):

> [!NOTE]
> Any component or element that accepts a `className` prop can also use
the `css` prop. The styles supplied to the `css` prop are evaluated and
the computed class name is applied to the `className` prop.

Components that are safe to migrate from `style` to `css`:
- React elements
- React components exposed by EUI (they all support a `className` prop
and the [Babel
plugin](https://www.npmjs.com/package/@emotion/babel-preset-css-prop) is
enabled throughout Kibana)

Components where styling might break:
- Custom component we wrote that currently don't accept a `className`
prop.
- Specific 3P components that pass in a `style` prop to our components
i.e. `react-window` (it calculates the dynamic position of all
virtualized elements and hides the ones outside the visible fold)
- Specific 3P components that expect a `style` prop to be styled i.e.
charts, etc.

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Identify risks

Main risk is breaking the UI by not applying or incorrectly applying the
styling. This was explained in the "How to review" section above. The
risk has **low severity** though. The mitigation plan will simply be
reverting the commit asap not to affect production and test locally
until it's fixed./
  • Loading branch information
albertoblaz authored Dec 2, 2024
1 parent f21fdaf commit 6aaecd0
Show file tree
Hide file tree
Showing 91 changed files with 196 additions and 198 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const MetricVisValue = (props: MetricVisValueProps) => {
if (onFilter) {
return (
<button
style={{ display: 'block' }}
css={{ display: 'block' }}
onClick={() => onFilter()}
title={i18n.translate('expressionLegacyMetricVis.filterTitle', {
defaultMessage: 'Click to filter by field',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ const buildFilterEvent = (rowIdx: number, columnIdx: number, table: Datatable) =
const getIcon =
(type: string) =>
({ width, height, color }: { width: number; height: number; color: string }) =>
<EuiIcon type={type} width={width} height={height} fill={color} style={{ width, height }} />;
<EuiIcon type={type} fill={color} style={{ width, height }} />;

export interface MetricVisComponentProps {
data: Datatable;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export const getLegendActions = (
ref={ref}
role="button"
aria-pressed="false"
style={{
css={{
display: 'flex',
justifyContent: 'center',
alignItems: 'center',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export const LegendActionPopover: React.FunctionComponent<LegendActionPopoverPro
ref={ref}
role="button"
aria-pressed="false"
style={{
css={{
display: 'flex',
justifyContent: 'center',
alignItems: 'center',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export function MarkerBody({
return (
<div
className="eui-textTruncate"
style={{ maxWidth: LINES_MARKER_SIZE * 3 }}
css={{ maxWidth: LINES_MARKER_SIZE * 3 }}
data-test-subj="xyVisAnnotationText"
>
{label}
Expand All @@ -123,13 +123,13 @@ export function MarkerBody({
<div
className="xyDecorationRotatedWrapper"
data-test-subj="xyVisAnnotationText"
style={{
css={{
width: LINES_MARKER_SIZE,
}}
>
<div
className="eui-textTruncate xyDecorationRotatedWrapper__label"
style={{
css={{
maxWidth: LINES_MARKER_SIZE * 3,
}}
>
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/charts/public/static/components/warnings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export function Warnings({
)
}
>
<div style={{ maxWidth: 512 }}>
<div css={{ maxWidth: 512 }}>
{warnings.map((w, i) => (
<React.Fragment key={i}>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export const HelpPopover = ({ button, isOpen, closePopover, resetTour }: HelpPop

<EuiSpacer size="s" />

<EuiText style={{ width: 300 }} color="subdued" size="s">
<EuiText css={{ width: 300 }} color="subdued" size="s">
<p>
{i18n.translate('console.helpPopover.description', {
defaultMessage:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export const Editor = memo(({ loading, inputEditorValue, setInputEditorValue }:
</div>
) : null}
<EuiResizableContainer
style={{ height: '100%' }}
css={{ height: '100%' }}
direction={isVerticalLayout ? 'vertical' : 'horizontal'}
onPanelWidthChange={(sizes) => onPanelSizeChange(sizes)}
data-test-subj="consoleEditorContainer"
Expand All @@ -152,13 +152,13 @@ export const Editor = memo(({ loading, inputEditorValue, setInputEditorValue }:
grow={true}
borderRadius="none"
hasShadow={false}
style={{ height: '100%' }}
css={{ height: '100%' }}
>
<EuiSplitPanel.Inner
paddingSize="none"
grow={true}
className="consoleEditorPanel"
style={{ top: 0, height: 'calc(100% - 40px)' }}
css={{ top: 0, height: 'calc(100% - 40px)' }}
>
{loading ? (
<EditorContentSpinner />
Expand Down Expand Up @@ -210,7 +210,7 @@ export const Editor = memo(({ loading, inputEditorValue, setInputEditorValue }:
tabIndex={0}
paddingSize="none"
>
<EuiSplitPanel.Outer borderRadius="none" hasShadow={false} style={{ height: '100%' }}>
<EuiSplitPanel.Outer borderRadius="none" hasShadow={false} css={{ height: '100%' }}>
<EuiSplitPanel.Inner
paddingSize="none"
css={{ alignContent: 'center', top: 0, height: 'calc(100% - 40px)' }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export function History() {
data-test-subj="consoleHistoryPanel"
>
<EuiResizableContainer
style={{ height: '100%' }}
css={{ height: '100%' }}
direction={isVerticalLayout ? 'vertical' : 'horizontal'}
>
{(EuiResizablePanel, EuiResizableButton) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const OptionsListPopover = () => {
<div
data-test-subj={`optionsList-control-available-options`}
data-option-count={loading ? 0 : Object.keys(availableOptions ?? {}).length}
style={{ width: '100%', height: '100%' }}
css={{ width: '100%', height: '100%' }}
>
<OptionsListPopoverSuggestions showOnlySelected={showOnlySelected} />
{!showOnlySelected && invalidSelections && invalidSelections.size !== 0 && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export const OptionsListPopoverFooter = () => {
`}
>
{loading && (
<div style={{ position: 'absolute', width: '100%' }}>
<div css={{ position: 'absolute', width: '100%' }}>
<EuiProgress
data-test-subj="optionsList-control-popover-loading"
size="xs"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const SearchSessionName: React.FC<SearchSessionNameProps> = ({ name, edit
justifyContent={'spaceBetween'}
gutterSize={'none'}
// padding to align with compressed input size
style={{ paddingTop: 4, paddingBottom: 4 }}
css={{ paddingTop: 4, paddingBottom: 4 }}
>
<EuiText size={'s'} className={'eui-textTruncate'}>
<h4 className={'eui-textTruncate'}>{name}</h4>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const AdvancedParamsSection = ({ children, defaultVisible = false }: Prop
})}
</EuiButtonEmpty>

<div style={{ display: isVisible ? 'block' : 'none' }} data-test-subj="advancedSettings">
<div css={{ display: isVisible ? 'block' : 'none' }} data-test-subj="advancedSettings">
<EuiSpacer size="m" />
{/* We ned to wrap the children inside a "div" to have our css :first-child rule */}
<div>{children}</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ interface TypeFieldProps {
}

const standardSelectItem = (
<EuiDescriptionList style={{ whiteSpace: 'nowrap' }} data-test-subj="standardType">
<EuiDescriptionList css={{ whiteSpace: 'nowrap' }} data-test-subj="standardType">
<EuiDescriptionListTitle>
<FormattedMessage
id="indexPatternEditor.typeSelect.standardTitle"
Expand All @@ -49,7 +49,7 @@ const standardSelectItem = (
);

const rollupSelectItem = (
<EuiDescriptionList style={{ whiteSpace: 'nowrap' }} data-test-subj="rollupType">
<EuiDescriptionList css={{ whiteSpace: 'nowrap' }} data-test-subj="rollupType">
<EuiDescriptionListTitle>
<FormattedMessage
id="indexPatternEditor.typeSelect.rollupTitle"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const AdvancedParametersSection = ({ children }: Props) => {
})}
</EuiButtonEmpty>

<div style={{ display: isVisible ? 'block' : 'none' }} data-test-subj="advancedSettings">
<div css={{ display: isVisible ? 'block' : 'none' }} data-test-subj="advancedSettings">
<EuiSpacer size="m" />
{/* We ned to wrap the children inside a "div" to have our css :first-child rule */}
<div>{children}</div>
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export class StaticLookupFormatEditor extends DefaultFormatEditor<StaticLookupFo

return (
<Fragment>
<EuiBasicTable items={items} columns={columns} style={{ maxWidth: '400px' }} />
<EuiBasicTable items={items} columns={columns} css={{ maxWidth: '400px' }} />
<EuiSpacer size="m" />
<EuiButton
iconType="plusInCircle"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export class FormatEditor extends PureComponent<FormatEditorProps, FormatEditorS
fallback={
// We specify minHeight to avoid too mitigate layout shifts while loading an editor
// ~430 corresponds to "4 lines" of EuiSkeletonText
<div style={{ minHeight: 430, marginTop: 8 }}>
<div css={{ minHeight: 430, marginTop: 8 }}>
<EuiDelayRender>
<EuiSkeletonText lines={4} />
</EuiDelayRender>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export const FieldPreview = () => {
{/* List of other fields in the document */}
<EuiResizeObserver onResize={onFieldListResize}>
{(resizeRef) => (
<div ref={resizeRef} style={{ flex: 1 }}>
<div ref={resizeRef} css={{ flex: 1 }}>
<PreviewFieldList
height={fieldListHeight}
clearSearch={() => setSearchValue('')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { EuiEmptyPrompt, EuiText, EuiTextColor, EuiFlexGroup, EuiFlexItem } from

export const FieldPreviewEmptyPrompt = () => {
return (
<EuiFlexGroup style={{ height: '100%' }} data-test-subj="emptyPrompt">
<EuiFlexGroup css={{ height: '100%' }} data-test-subj="emptyPrompt">
<EuiFlexItem>
<EuiEmptyPrompt
iconType="inspect"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export const AddDataPrompt: React.FC<AddDataPromptComponentProps> = ({
return { default: DataViewIllustration };
})
),
<EuiPanel color="subdued" style={{ width: 226, height: 206 }} />
<EuiPanel color="subdued" css={{ width: 226, height: 206 }} />
);

return (
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const Header = withRouter(({ indexPatternId, history }: HeaderProps) => {
defaultMessage="Scripted fields can be used in visualizations and displayed in documents. However, they cannot be searched."
/>
<br />
<EuiIcon type="warning" color="warning" style={{ marginRight: '4px' }} />
<EuiIcon type="warning" color="warning" css={{ marginRight: '4px' }} />
<FormattedMessage
id="indexPatternManagement.editIndexPattern.deprecation"
tagName="span"
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class FieldFormatEditor extends PureComponent<
fallback={
// We specify minHeight to avoid too mitigate layout shifts while loading an editor
// ~430 corresponds to "4 lines" of EuiSkeletonText
<div style={{ minHeight: 430, marginTop: 8 }}>
<div css={{ minHeight: 430, marginTop: 8 }}>
<EuiDelayRender>
<EuiSkeletonText lines={4} />
</EuiDelayRender>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ export const IndexPatternTable = ({
return (
<div data-test-subj="indexPatternTable" role="region" aria-label={title}>
{isLoadingDataState ? (
<div style={{ display: 'flex', justifyContent: 'center' }}>
<div css={{ display: 'flex', justifyContent: 'center' }}>
<EuiLoadingSpinner size="xxl" />
</div>
) : (
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/dev_tools/public/application.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function DevToolsWrapper({

return (
<main className="devApp">
<EuiTabs style={{ paddingLeft: euiThemeVars.euiSizeS }} size="l">
<EuiTabs css={{ paddingLeft: euiThemeVars.euiSizeS }} size="l">
{devTools.map((currentDevTool) => (
<EuiTab
key={currentDevTool.id}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default function ESQLToDataViewTransitionModal({
return (
<EuiModal
onClose={() => onClose()}
style={{ width: 700 }}
css={{ width: 700 }}
data-test-subj="discover-esql-to-dataview-modal"
>
<EuiModalHeader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const TotalDocuments = ({
<EuiText
grow={false}
size="s"
style={{ paddingRight: 2 }}
css={{ paddingRight: 2 }}
data-test-subj="savedSearchTotalDocuments"
>
{isEsqlMode ? (
Expand Down
Loading

0 comments on commit 6aaecd0

Please sign in to comment.