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

[EuiTabs] More Emotion cleanup #6336

Merged
merged 11 commits into from
Nov 7, 2022
Merged

[EuiTabs] More Emotion cleanup #6336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bf4b747
Remove mounted snapshot
cee-chen Nov 1, 2022
0559db6
Emotion cleanup - remove unnecessary `calc`s
cee-chen Nov 1, 2022
72e9fed
CSS cleanup - remove incorrect text color and unnecessary extra hover…
cee-chen Nov 1, 2022
b50a816
CSS - misc logical property conversion
cee-chen Nov 1, 2022
2033da5
Flatten/fix disabled selected selector
cee-chen Nov 1, 2022
9ed121a
Revert removed `__append/prepend` classNames
cee-chen Nov 1, 2022
d7f612e
DRY out tab sizes types/consts
cee-chen Nov 1, 2022
56da139
Add missing `shouldRenderCustomStyles` tests to sub components
cee-chen Nov 1, 2022
bb66f3f
[misc] rename Emotion vars to match other components/conversions
cee-chen Nov 1, 2022
39712c9
Remove unnecessary padding sizes on `euiTab`
cee-chen Nov 1, 2022
3e8a548
changelog
cee-chen Nov 1, 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
4 changes: 2 additions & 2 deletions src/components/page/page_header/__snapshots__/page_header.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ exports[`EuiPageHeader props page content props are passed down is rendered 1`]
>
<button
aria-selected="true"
class="euiTab euiTab-isSelected emotion-euiTab-selected-l"
class="euiTab euiTab-isSelected emotion-euiTab-selected"
role="tab"
type="button"
>
Expand All @@ -342,7 +342,7 @@ exports[`EuiPageHeader props page content props are passed down is rendered 1`]
</button>
<button
aria-selected="false"
class="euiTab emotion-euiTab-l"
class="euiTab emotion-euiTab"
role="tab"
type="button"
>
Expand Down
12 changes: 6 additions & 6 deletions src/components/page/page_header/__snapshots__/page_header_content.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ exports[`EuiPageHeaderContent props children is rendered even if content props a
>
<button
aria-selected="true"
class="euiTab euiTab-isSelected emotion-euiTab-selected-l"
class="euiTab euiTab-isSelected emotion-euiTab-selected"
role="tab"
type="button"
>
Expand All @@ -302,7 +302,7 @@ exports[`EuiPageHeaderContent props children is rendered even if content props a
</button>
<button
aria-selected="false"
class="euiTab emotion-euiTab-l"
class="euiTab emotion-euiTab"
role="tab"
type="button"
>
Expand Down Expand Up @@ -579,7 +579,7 @@ exports[`EuiPageHeaderContent props tabs is rendered 1`] = `
>
<button
aria-selected="true"
class="euiTab euiTab-isSelected emotion-euiTab-selected-xl"
class="euiTab euiTab-isSelected emotion-euiTab-selected"
role="tab"
type="button"
>
Expand All @@ -591,7 +591,7 @@ exports[`EuiPageHeaderContent props tabs is rendered 1`] = `
</button>
<button
aria-selected="false"
class="euiTab emotion-euiTab-xl"
class="euiTab emotion-euiTab"
role="tab"
type="button"
>
Expand Down Expand Up @@ -630,7 +630,7 @@ exports[`EuiPageHeaderContent props tabs is rendered with tabsProps 1`] = `
>
<button
aria-selected="true"
class="euiTab euiTab-isSelected emotion-euiTab-selected-xl"
class="euiTab euiTab-isSelected emotion-euiTab-selected"
role="tab"
type="button"
>
Expand All @@ -642,7 +642,7 @@ exports[`EuiPageHeaderContent props tabs is rendered with tabsProps 1`] = `
</button>
<button
aria-selected="false"
class="euiTab emotion-euiTab-xl"
class="euiTab emotion-euiTab"
role="tab"
type="button"
>
Expand Down
24 changes: 22 additions & 2 deletions src/components/tabs/__snapshots__/tab.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ exports[`EuiTab props append is rendered 1`] = `
role="tab"
type="button"
>
<span>
<span
class="euiTab__prepend"
>
Append
</span>
<span
Expand All @@ -18,6 +20,22 @@ exports[`EuiTab props append is rendered 1`] = `
</button>
`;

exports[`EuiTab props disabled and selected 1`] = `
<button
aria-selected="true"
class="euiTab euiTab-isSelected emotion-euiTab-disabled-selected"
disabled=""
role="tab"
type="button"
>
<span
class="euiTab__content emotion-euiTab__content-selected-disabled"
>
Click Me
</span>
</button>
`;

exports[`EuiTab props disabled is rendered 1`] = `
<button
aria-selected="false"
Expand Down Expand Up @@ -91,7 +109,9 @@ exports[`EuiTab props prepend is rendered 1`] = `
role="tab"
type="button"
>
<span>
<span
class="euiTab__prepend"
>
Prepend
</span>
<span
Expand Down
53 changes: 21 additions & 32 deletions src/components/tabs/tab.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { css } from '@emotion/react';
import { logicalCSS, mathWithUnits } from '../../global_styling';
import { UseEuiTheme } from '../../services';
import { euiTitle } from '../title/title.styles';

Expand All @@ -19,44 +20,32 @@ export const euiTabStyles = ({ euiTheme }: UseEuiTheme) => {
align-items: center;
font-weight: ${euiTheme.font.weight.semiBold};
gap: ${euiTheme.size.s};
${logicalCSS('padding-vertical', 0)}
${logicalCSS('padding-horizontal', euiTheme.size.xs)}

&:focus {
background-color: transparent;
outline-offset: -${euiTheme.focus.width};
}
`,
// sizes
s: css`
padding: 0 ${euiTheme.size.xs};
`,
m: css`
padding: 0 ${euiTheme.size.xs};
`,
l: css`
padding: 0 ${euiTheme.size.xs};
`,
xl: css`
padding: ${euiTheme.size.s} ${euiTheme.size.xs};
`,
Comment on lines -28 to -40
Copy link
Contributor Author

@cee-chen cee-chen Nov 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@miukimiu stop me if I'm reading the old amsterdam overrides SCSS wrong but it looked to me like in Amsterdam all paddings were supposed to be 0 xs and as such we don't need a sizes modifier.

The playground in https://eui.elastic.co/v67.0.0/#/navigation/tabs (pre-Emotion-conversion) appears to confirm this

Copy link
Contributor

Choose a reason for hiding this comment

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

I confirmed by looking into the old file and you're right.

// variations
expanded: css`
flex-basis: 0%;
flex-grow: 1;
justify-content: center;
`,
selected: css`
box-shadow: inset 0 calc(${euiTheme.border.width.thick} * -1) 0
${euiTheme.colors.primary};
`,
disabled: css`
cursor: not-allowed;
color: ${euiTheme.colors.disabledText};

.euiTab.euiTab__isSelected {
box-shadow: inset 0 calc(${euiTheme.border.width.thick} * -1) 0
${euiTheme.colors.disabledText};
}
box-shadow: inset 0 -${euiTheme.border.width.thick} 0 ${euiTheme.colors.primary};
`,
disabled: {
disabled: css`
cursor: not-allowed;
color: ${euiTheme.colors.disabledText};
`,
selected: css`
box-shadow: inset 0 -${euiTheme.border.width.thick} 0 ${euiTheme.colors.disabledText};
`,
},
};
};

Expand All @@ -65,8 +54,6 @@ export const euiTabContentStyles = (euiThemeContext: UseEuiTheme) => {

return {
euiTab__content: css`
color: ${euiTheme.colors.disabledText};

&:hover {
text-decoration: none;
}
Expand All @@ -82,22 +69,24 @@ export const euiTabContentStyles = (euiThemeContext: UseEuiTheme) => {
`,
l: css`
${euiTitle(euiThemeContext, 'xs')};
line-height: calc(${euiTheme.size.xl} + ${euiTheme.size.s});
line-height: ${mathWithUnits(
[euiTheme.size.xl, euiTheme.size.s],
(x, y) => x + y
)};
`,
xl: css`
${euiTitle(euiThemeContext, 's')};
line-height: calc(${euiTheme.size.xxxl} + ${euiTheme.size.s});
line-height: ${mathWithUnits(
[euiTheme.size.xxxl, euiTheme.size.s],
(x, y) => x + y
)};
`,
// variations
selected: css`
color: ${euiTheme.colors.primaryText};
`,
disabled: css`
color: ${euiTheme.colors.disabledText};

&:hover {
text-decoration: none;
}
`,
};
};
13 changes: 13 additions & 0 deletions src/components/tabs/tab.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@

import React from 'react';
import { render, shallow } from 'enzyme';
import { shouldRenderCustomStyles } from '../../test/internal';
import { requiredProps } from '../../test/required_props';

import { EuiTab } from './tab';

describe('EuiTab', () => {
shouldRenderCustomStyles(<EuiTab onClick={() => {}}>children</EuiTab>);

describe('props', () => {
describe('onClick', () => {
test('renders button', () => {
Expand Down Expand Up @@ -57,6 +60,16 @@ describe('EuiTab', () => {
expect(render(component)).toMatchSnapshot();
});

test('disabled and selected', () => {
const component = render(
<EuiTab disabled isSelected>
Click Me
</EuiTab>
);

expect(component).toMatchSnapshot();
});

test('prepend is rendered', () => {
const component = (
<EuiTab onClick={() => {}} prepend="Prepend">
Expand Down
16 changes: 8 additions & 8 deletions src/components/tabs/tab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { getSecureRelForTarget, useEuiTheme } from '../../services';
import { validateHref } from '../../services/security/href_validator';

import { euiTabStyles, euiTabContentStyles } from './tab.styles';
import { EuiTabsProps, EuiTabSizes } from './tabs';
import { EuiTabsProps, EuiTabsSizes } from './tabs';

export interface EuiTabProps extends CommonProps {
isSelected?: boolean;
Expand All @@ -43,7 +43,7 @@ export interface EuiTabProps extends CommonProps {
* Sizes affect both font size and overall size.
* Only use the `xl` size when displayed as page titles.
*/
size?: EuiTabSizes;
size?: EuiTabsSizes;
}

type EuiTabPropsForAnchor = EuiTabProps &
Expand Down Expand Up @@ -86,9 +86,8 @@ export const EuiTab: FunctionComponent<Props> = ({
const cssTabStyles = [
tabStyles.euiTab,
expand && tabStyles.expanded,
isSelected && tabStyles.selected,
disabled && tabStyles.disabled,
size && tabStyles[size],
disabled && tabStyles.disabled.disabled,
isSelected && (disabled ? tabStyles.disabled.selected : tabStyles.selected),
];

const tabContentStyles = euiTabContentStyles(euiTheme);
Expand All @@ -99,12 +98,13 @@ export const EuiTab: FunctionComponent<Props> = ({
disabled && tabContentStyles.disabled,
];

const prependNode = prepend && <span>{prepend}</span>;
const appendNode = append && <span>{append}</span>;
const prependNode = prepend && (
<span className="euiTab__prepend">{prepend}</span>
);
const appendNode = append && <span className="euiTab__append">{append}</span>;

// <a> elements don't respect the `disabled` attribute. So if we're disabled, we'll just pretend
// this is a button and piggyback off its disabled styles.

if (href && !disabled) {
const secureRel = getSecureRelForTarget({ href, target, rel });

Expand Down
Loading