Skip to content

Commit

Permalink
[EuiPopoverFooter] Fix buggy panel padding size context (#6698)
Browse files Browse the repository at this point in the history
* [REVERT ME] Reproduction of bug

* Fix incorrect context usage

This component only should be *setting* context, not *using* it (which is causing incorrect inheritance from nested popovers)

* [misc cleanup] remove unnecessary arg passed to `euiPopoverFooterStyles`

- use Emotion keys and an internal helper util instead

+ comment cleanup so

* update downstream snapshots

* changelog

* Revert "[REVERT ME] Reproduction of bug"

This reverts commit 3d6f396.
  • Loading branch information
cee-chen committed Apr 6, 2023
1 parent 34748f9 commit d6c8e10
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Array [
</div>
</div>,
<div
class="euiPopoverFooter emotion-euiPopoverFooter-l"
class="euiPopoverFooter emotion-euiPopoverFooter-l-l"
>
<div
class="euiFlexGroup emotion-euiFlexGroup-wrap-s-flexStart-stretch-row"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ exports[`useDataGridColumnSelector columnSelector renders a toolbar button/popov
</div>
</div>
<div
class="euiPopoverFooter emotion-euiPopoverFooter-s"
class="euiPopoverFooter emotion-euiPopoverFooter-s-s"
>
<div
class="euiFlexGroup emotion-euiFlexGroup-s-spaceBetween-stretch-row"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ exports[`useDataGridColumnSorting columnSorting renders a toolbar button/popover
</div>
</div>
<div
class="euiPopoverFooter emotion-euiPopoverFooter-s"
class="euiPopoverFooter emotion-euiPopoverFooter-s-s"
>
<div
class="euiFlexGroup emotion-euiFlexGroup-m-spaceBetween-stretch-row"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,43 +3,43 @@
exports[`EuiPopoverFooter is rendered 1`] = `
<div
aria-label="aria-label"
class="euiPopoverFooter testClass1 testClass2 emotion-euiPopoverFooter-l"
class="euiPopoverFooter testClass1 testClass2 emotion-euiPopoverFooter-l-l"
data-test-subj="test subject string"
/>
`;

exports[`EuiPopoverFooter props paddingSize l is rendered 1`] = `
<div
class="euiPopoverFooter emotion-euiPopoverFooter-l"
class="euiPopoverFooter emotion-euiPopoverFooter-l-l"
/>
`;

exports[`EuiPopoverFooter props paddingSize m is rendered 1`] = `
<div
class="euiPopoverFooter emotion-euiPopoverFooter-m"
class="euiPopoverFooter emotion-euiPopoverFooter-l-m"
/>
`;

exports[`EuiPopoverFooter props paddingSize none is rendered 1`] = `
<div
class="euiPopoverFooter emotion-euiPopoverFooter"
class="euiPopoverFooter emotion-euiPopoverFooter-l"
/>
`;

exports[`EuiPopoverFooter props paddingSize s is rendered 1`] = `
<div
class="euiPopoverFooter emotion-euiPopoverFooter-s"
class="euiPopoverFooter emotion-euiPopoverFooter-l-s"
/>
`;

exports[`EuiPopoverFooter props paddingSize xl is rendered 1`] = `
<div
class="euiPopoverFooter emotion-euiPopoverFooter-xl"
class="euiPopoverFooter emotion-euiPopoverFooter-l-xl"
/>
`;

exports[`EuiPopoverFooter props paddingSize xs is rendered 1`] = `
<div
class="euiPopoverFooter emotion-euiPopoverFooter-xs"
class="euiPopoverFooter emotion-euiPopoverFooter-l-xs"
/>
`;
46 changes: 33 additions & 13 deletions src/components/popover/popover_footer.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,46 @@ import {
} from '../../global_styling';
import { UseEuiTheme } from '../../services';

export const euiPopoverFooterStyles = (
euiThemeContext: UseEuiTheme,
panelPadding: EuiPaddingSize
) => {
export const euiPopoverFooterStyles = (euiThemeContext: UseEuiTheme) => {
const { euiTheme } = euiThemeContext;
// If the popover's containing panel has padding applied,
// ensure the title expands to cover that padding and
const panelPaddingSize = euiPaddingSize(euiThemeContext, panelPadding);

return {
// Base
euiPopoverFooter: css`
${euiFontSize(euiThemeContext, 's')};
${logicalCSS('border-top', euiTheme.border.thin)};
// Negative margins for panel padding
${logicalShorthandCSS(
'margin',
`${panelPaddingSize} -${panelPaddingSize} -${panelPaddingSize}`
)}
`,
// If the popover's containing panel has padding applied,
// ensure the title expands to cover that padding via negative margins
panelPaddingSizes: {
none: css``,
xs: css`
${panelPaddingOffset(euiThemeContext, 'xs')}
`,
s: css`
${panelPaddingOffset(euiThemeContext, 's')}
`,
m: css`
${panelPaddingOffset(euiThemeContext, 'm')}
`,
l: css`
${panelPaddingOffset(euiThemeContext, 'l')}
`,
xl: css`
${panelPaddingOffset(euiThemeContext, 'xl')}
`,
},
};
};

export const panelPaddingOffset = (
euiThemeContext: UseEuiTheme,
size: EuiPaddingSize
) => {
const panelPaddingSize = euiPaddingSize(euiThemeContext, size);

return logicalShorthandCSS(
'margin',
`${panelPaddingSize} -${panelPaddingSize} -${panelPaddingSize}`
);
};
6 changes: 3 additions & 3 deletions src/components/popover/popover_footer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ export const EuiPopoverFooter: EuiPopoverFooterProps = ({
}) => {
const { paddingSize: panelPadding } = useContext(EuiPopoverPanelContext);
const euiTheme = useEuiTheme();
const styles = euiPopoverFooterStyles(euiTheme, panelPadding);
const styles = euiPopoverFooterStyles(euiTheme);
const paddingStyles = useEuiPaddingCSS();
const cssStyles = [
styles.euiPopoverFooter,
// If a paddingSize is not directly provided, inherit from the EuiPopoverPanel
paddingStyles[paddingSize || panelPadding],
styles.panelPaddingSizes[panelPadding],
paddingStyles[paddingSize || panelPadding], // If a paddingSize is not directly provided, inherit from the EuiPopoverPanel
];

const classes = classNames('euiPopoverFooter', className);
Expand Down
17 changes: 8 additions & 9 deletions src/components/popover/popover_panel/_popover_panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@
* Side Public License, v 1.
*/

import React, { createContext, FunctionComponent, useContext } from 'react';
import React, { createContext, FunctionComponent } from 'react';
import classNames from 'classnames';
import { useEuiTheme } from '../../../services';
import { EuiPaddingSize } from '../../../global_styling';
import { EuiPanel, _EuiPanelDivlike } from '../../panel/panel';
import { EuiPopoverArrowPositions } from '../popover_arrow';
import { euiPopoverPanelStyles } from './_popover_panel.styles';

interface ContextShape {
paddingSize: EuiPaddingSize;
}
const DEFAULT_PANEL_PADDING_SIZE = 'l';

export const EuiPopoverPanelContext = createContext<ContextShape>({
paddingSize: 'l',
export const EuiPopoverPanelContext = createContext<{
paddingSize: EuiPaddingSize;
}>({
paddingSize: DEFAULT_PANEL_PADDING_SIZE,
});

export type EuiPopoverPanelProps = _EuiPanelDivlike;
Expand All @@ -46,8 +46,7 @@ export const EuiPopoverPanel: FunctionComponent<
position,
...rest
}) => {
const panelContext = useContext(EuiPopoverPanelContext);
if (rest.paddingSize) panelContext.paddingSize = rest.paddingSize;
const { paddingSize = DEFAULT_PANEL_PADDING_SIZE } = rest;

const euiThemeContext = useEuiTheme();
// Using BEM child class for BWC
Expand Down Expand Up @@ -77,7 +76,7 @@ export const EuiPopoverPanel: FunctionComponent<
}

return (
<EuiPopoverPanelContext.Provider value={panelContext}>
<EuiPopoverPanelContext.Provider value={{ paddingSize }}>
<EuiPanel
className={classes}
css={panelCSS}
Expand Down
12 changes: 6 additions & 6 deletions src/components/tour/__snapshots__/tour_step.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ exports[`EuiTourStep accepts an array of buttons in the footerAction prop 1`] =
You are here
</div>
<div
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-euiTourFooter"
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-m-euiTourFooter"
>
<div
class="euiFlexGroup emotion-euiFlexGroup-l-flexEnd-center-row"
Expand Down Expand Up @@ -174,7 +174,7 @@ exports[`EuiTourStep can change the minWidth and maxWidth 1`] = `
You are here
</div>
<div
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-euiTourFooter"
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-m-euiTourFooter"
>
<div
class="euiFlexGroup emotion-euiFlexGroup-l-flexEnd-center-row"
Expand Down Expand Up @@ -277,7 +277,7 @@ exports[`EuiTourStep can have subtitle 1`] = `
You are here
</div>
<div
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-euiTourFooter"
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-m-euiTourFooter"
>
<div
class="euiFlexGroup emotion-euiFlexGroup-l-flexEnd-center-row"
Expand Down Expand Up @@ -375,7 +375,7 @@ exports[`EuiTourStep can override the footer action 1`] = `
You are here
</div>
<div
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-euiTourFooter"
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-m-euiTourFooter"
>
<div
class="euiFlexGroup emotion-euiFlexGroup-l-flexEnd-center-row"
Expand Down Expand Up @@ -457,7 +457,7 @@ exports[`EuiTourStep can turn off the beacon 1`] = `
You are here
</div>
<div
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-euiTourFooter"
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-m-euiTourFooter"
>
<div
class="euiFlexGroup emotion-euiFlexGroup-l-flexEnd-center-row"
Expand Down Expand Up @@ -555,7 +555,7 @@ exports[`EuiTourStep is rendered 1`] = `
You are here
</div>
<div
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-euiTourFooter"
class="euiPopoverFooter euiTourFooter emotion-euiPopoverFooter-m-m-euiTourFooter"
>
<div
class="euiFlexGroup emotion-euiFlexGroup-l-flexEnd-center-row"
Expand Down
3 changes: 3 additions & 0 deletions upcoming_changelogs/6698.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed an `EuiPopoverFooter` bug causing nested popovers within popovers (note: not a recommended use-case) to unintentionally override its panel padding size inherited from context

0 comments on commit d6c8e10

Please sign in to comment.