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.

Sign up for GitHub

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

Jump to bottom

[EuiButton] Fix multiple issues around internal EuiButtonDisplay components #6332

Merged
merged 9 commits into from
Oct 31, 2022
Merged

[EuiButton] Fix multiple issues around internal EuiButtonDisplay components #6332

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
210e1d6
Fix text wrapper not being used for EuiI18n 'text' that pass textProps
cee-chen Oct 28, 2022
5bef472
Fix unnecessary textProps spread causing false positive for undefined…
cee-chen Oct 28, 2022
8252261
Fix minWidth inline style not applying for 0px + use logical property
cee-chen Oct 28, 2022
a0c2e47
[opinionated] iconSide change: prefer DOM order over CSS order
cee-chen Oct 28, 2022
686b797
[cleanup] remove unnecessary/unused CSS
cee-chen Oct 28, 2022
13d9805
Write more unit tests for EuiButtonDisplayContent
cee-chen Oct 28, 2022
c56336f
Fix EuiLoadingSpinner to not pass wildcard color values into Emotion
cee-chen Oct 28, 2022
8b51bd7
write more/missing unit tests for logic within internal EuiButtonDisplay
cee-chen Oct 28, 2022
0fdfb69
changelog
cee-chen Oct 28, 2022
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
15 changes: 8 additions & 7 deletions src/components/button/__snapshots__/button.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ exports[`EuiButton props iconSide left is rendered 1`] = `
type="button"
>
<span
class="emotion-euiButtonDisplayContent-left"
class="emotion-euiButtonDisplayContent"
>
<span
class="emotion-euiButtonDisplayContent__icon-m"
Expand All @@ -178,18 +178,18 @@ exports[`EuiButton props iconSide right is rendered 1`] = `
type="button"
>
<span
class="emotion-euiButtonDisplayContent-right"
class="emotion-euiButtonDisplayContent"
>
<span
class="emotion-euiButtonDisplayContent__icon-m"
color="inherit"
data-euiicon-type="user"
/>
<span
class="eui-textTruncate"
>
Content
</span>
<span
class="emotion-euiButtonDisplayContent__icon-m"
color="inherit"
data-euiicon-type="user"
/>
</span>
</button>
`;
Expand Down Expand Up @@ -336,6 +336,7 @@ exports[`EuiButton props isSelected is rendered as true 1`] = `
exports[`EuiButton props minWidth is rendered 1`] = `
<button
class="euiButton emotion-euiButtonDisplay-m-m-base-primary"
style="min-inline-size:0"
type="button"
>
<span
Expand Down
2 changes: 1 addition & 1 deletion src/components/button/button_display/_button_display.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export const EuiButtonDisplay = forwardRef<HTMLElement, EuiButtonDisplayProps>(
{
children,
iconType,
iconSide,
iconSide = 'left',
iconSize,
size = 'm',
isDisabled,
Expand Down
5 changes: 0 additions & 5 deletions src/components/button/button_display/_button_display_content.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ export const euiButtonDisplayContentStyles = ({ euiTheme }: UseEuiTheme) => ({
vertical-align: middle;
gap: ${euiTheme.size.s};
`,
// Icon side
left: css``,
right: css`
flex-direction: row-reverse;
`,
euiButtonDisplayContent__spinner: css`
flex-shrink: 0;
`,
Expand Down
8 changes: 3 additions & 5 deletions src/components/button/button_display/_button_display_content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,7 @@ export const EuiButtonDisplayContent: FunctionComponent<
const theme = useEuiTheme();
const styles = euiButtonDisplayContentStyles(theme);

const cssStyles = [
styles.euiButtonDisplayContent,
iconSide && styles[iconSide],
];
const cssStyles = [styles.euiButtonDisplayContent];
const cssSpinnerStyles = [styles.euiButtonDisplayContent__spinner];
const cssIconStyles = [
styles.euiButtonDisplayContent__icon,
Expand Down Expand Up @@ -110,7 +107,7 @@ export const EuiButtonDisplayContent: FunctionComponent<

return (
<span css={cssStyles} {...contentProps}>
{icon}
{iconSide === 'left' && icon}
Copy link
Contributor

Choose a reason for hiding this comment

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

This change makes a lot of sense! I like it. From what I can see in Kibana, the only things referencing the the side in the icon class name are snapshots, so I believe we're safe to move forward with this.

{isText || textProps ? (
<span
{...textProps}
Expand All @@ -121,6 +118,7 @@ export const EuiButtonDisplayContent: FunctionComponent<
) : (
children
)}
{iconSide === 'right' && icon}
</span>
);
};