Skip to content
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

Order Emotion styles before Sass styles #161592

Merged
merged 7 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ export const Template: FunctionComponent<Props> = ({
<meta name="color-scheme" content="light dark" />
{/* Inject EUI reset and global styles before all other component styles */}
<meta name={EUI_STYLES_GLOBAL} />
<Styles darkMode={darkMode} stylesheetPaths={stylesheetPaths} />
<meta name="emotion" />
<Styles darkMode={darkMode} stylesheetPaths={stylesheetPaths} />
{/* Inject stylesheets into the <head> before scripts so that KP plugins with bundled styles will override them */}
<meta name="add-styles-here" />
<meta name="add-scripts-here" />
Expand Down
3 changes: 1 addition & 2 deletions packages/kbn-storybook/templates/index.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<meta name="viewport" content="width=device-width, initial-scale=1" />

<meta name="eui-global" />
<meta name="emotion" />

<!-- Added for Kibana shared dependencies -->
<script>
Expand All @@ -26,8 +27,6 @@
<link id="eui-theme-css" href="kbn-ui-shared-deps-npm.v8.light.css" rel="stylesheet" />
<!-- -->

<meta name="emotion" />

<% if (typeof headHtmlSnippet !== 'undefined') { %> <%= headHtmlSnippet %> <% } %> <%
htmlWebpackPlugin.files.css.forEach(file => { %>
<link href="<%= file %>" rel="stylesheet" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const PanelsResizable = ({
[]
);
const resizeWithPortalsHackButtonCss = css`
z-index: 3;
z-index: 3 !important; // !important can be removed once EuiResizableButton is converted to Emotion
`;
const resizeWithPortalsHackOverlayCss = css`
position: absolute;
Expand Down Expand Up @@ -164,7 +164,9 @@ export const PanelsResizable = ({

const { euiTheme } = useEuiTheme();
const buttonCss = css`
&& {
// The selectors here are intended to override EuiResizableButtons's Sass styles
// it can be removed once EuiResizableButton has been converted to Emotion
&.euiResizableButton.euiResizableButton--vertical {
margin-top: -${euiTheme.size.base};
margin-bottom: 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ export const ChangePointsTable: FC<ChangePointsTableProps> = ({
height: '80px',
truncateText: false,
valign: 'middle',
css: { display: 'block', padding: 0 },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything we can do about retaining the 0 padding here? As it is, the height of the rows increases with more unnecessary whitespace.

Before:
change_point_progress1

After:
change_point_progress2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦 Thanks so much for catching that Pete, I see what was happening now. I've restored the original CSS with an extra specificity selector so that the Emotion CSS overrides EUI's not-yet-converted Sass CSS: 60c382a

The TODO is for myself/EUI to remember to remove once EuiTable is converted to Emotion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making that change @cee-chen

render: (annotation: ChangePointAnnotation) => {
return <MiniChartPreview annotation={annotation} fieldConfig={fieldConfig} />;
},
Expand Down Expand Up @@ -319,7 +318,7 @@ export const MiniChartPreview: FC<ChartComponentProps> = ({ fieldConfig, annotat
});

return (
<div data-test-subj={'aiopChangePointPreviewChart'}>
<div data-test-subj={'aiopChangePointPreviewChart'} css={{ width: '100%' }}>
<EmbeddableComponent
id={`mini_changePointChart_${annotation.group ? annotation.group.value : annotation.label}`}
style={{ height: 80 }}
Expand Down