Skip to content

Commit

Permalink
fix: focus after delete, rename tokens, fix comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@savutsang
savutsang committed Apr 29, 2024
1 parent a19dd20 commit 575c8a6
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 83 deletions.
12 changes: 6 additions & 6 deletions packages/react/src/components/tabs/tab-button.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ describe('TabButton', () => {
const expectedButtonText = 'some text';
const wrapper = mountWithProviders(<TabButton {...focusedAndSelected}>{expectedButtonText}</TabButton>);

const tabPanel = getByTestId(wrapper, 'tabs-tab-text');
const tabPanel = getByTestId(wrapper, 'tab-text');

expect(tabPanel.prop('children')).toBe(expectedButtonText);
});

test('should not have a left icon in button when tab doesn\'t have a left icon name', () => {
const wrapper = mountWithProviders(<TabButton {...focusedAndSelected}>some text</TabButton>);

const tabButtonLeftIcon = findByTestId(wrapper, 'tabs-tab-left-icon');
const tabButtonLeftIcon = findByTestId(wrapper, 'tab-left-icon');

expect(tabButtonLeftIcon.length).toBe(0);
});
Expand All @@ -36,15 +36,15 @@ describe('TabButton', () => {
<TabButton {...focusedAndSelected} leftIcon={expectedLeftIcon}>some text</TabButton>,
);

const tabButtonLeftIcon = getByTestId(wrapper, 'tabs-tab-left-icon');
const tabButtonLeftIcon = getByTestId(wrapper, 'tab-left-icon');

expect(tabButtonLeftIcon.prop('name')).toBe(expectedLeftIcon);
});

test('should not have a right icon in button when tab doesn\'t have a right icon name', () => {
const wrapper = mountWithProviders(<TabButton {...focusedAndSelected}>some text</TabButton>);

const tabButtonRightIcon = findByTestId(wrapper, 'tabs-tab-right-icon');
const tabButtonRightIcon = findByTestId(wrapper, 'tab-right-icon');

expect(tabButtonRightIcon.length).toBe(0);
});
Expand All @@ -55,7 +55,7 @@ describe('TabButton', () => {
<TabButton {...focusedAndSelected} rightIcon={expectedRightIcon}>some text</TabButton>,
);

const tabButtonRightIcon = getByTestId(wrapper, 'tabs-tab-right-icon');
const tabButtonRightIcon = getByTestId(wrapper, 'tab-right-icon');

expect(tabButtonRightIcon.prop('name')).toBe(expectedRightIcon);
});
Expand All @@ -73,7 +73,7 @@ describe('TabButton', () => {
</TabButton>,
);

getByTestId(wrapper, 'tabs-tab-button').simulate('click');
getByTestId(wrapper, 'tab-button').simulate('click');

expect(expectedOnClickCall).toHaveBeenCalled();
});
Expand Down
32 changes: 16 additions & 16 deletions packages/react/src/components/tabs/tab-button.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const selectedIndicatorPosition = (global: boolean | undefined): string => (glob

const StyledButton = styled.button<{ $global?: boolean; $selected?: boolean; $removable?: boolean; }>`
align-items: center;
color: ${({ $selected, theme }) => ($selected ? theme.component['tabs-tab-selected-text-color'] : theme.component['tabs-tab-text-color'])};
color: ${({ $selected, theme }) => ($selected ? theme.component['tab-selected-text-color'] : theme.component['tab-text-color'])};
display: flex;
font-family: var(--font-family);
font-size: 0.875rem;
Expand All @@ -35,11 +35,11 @@ const StyledButton = styled.button<{ $global?: boolean; $selected?: boolean; $re
${({ $selected, theme }) => !$selected && css`
&:active {
color: ${theme.component['tabs-tab-active-text-color']};
color: ${theme.component['tab-active-text-color']};
font-weight: var(--font-semi-bold);
&::after {
background-color: ${theme.component['tabs-tab-active-indicator-color']} !important;
background-color: ${theme.component['tab-active-indicator-color']} !important;
}
}
`}
Expand All @@ -49,13 +49,13 @@ const StyledButton = styled.button<{ $global?: boolean; $selected?: boolean; $re
font-weight: var(--font-semi-bold);
&::after {
background-color: ${theme.component['tabs-tab-selected-indicator-color']};
background-color: ${theme.component['tab-selected-indicator-color']};
}
`}
`;

const StyledButtonIcon = styled(Icon)`
color: ${({ theme }) => theme.component['tabs-tab-icon-color']};
color: ${({ theme }) => theme.component['tab-icon-color']};
vertical-align: middle;
`;

Expand All @@ -67,23 +67,19 @@ const StyledTab = styled.div<{ $selected: boolean; }>`
&:hover {
${StyledButton} {
&::after {
background-color: ${theme.component['tabs-tab-hover-indicator-color']};
color: ${theme.component['tabs-tab-hover-text-color']};
background-color: ${theme.component['tab-hover-indicator-color']};
color: ${theme.component['tab-hover-text-color']};
}
}
}
`};
`;

const DeleteButton = styled(IconButton)`
min-height: var(--size-1x);
min-width: var(--size-1x);
padding: 0;
position: absolute;
right: var(--spacing-1halfx);
right: var(--spacing-1x);
top: 50%;
transform: translateY(-50%);
width: var(--size-1x);
`;

const ButtonLabel = styled.span`
Expand All @@ -107,7 +103,7 @@ interface TabButtonProps {
isSelected: boolean;
onClick(): void;
onRemove?(): void;
onKeyDown?(event: KeyboardEvent<HTMLButtonElement>): void;
onKeyDown?(event: KeyboardEvent<HTMLDivElement>): void;
}

export const TabButton = forwardRef(({
Expand All @@ -125,11 +121,15 @@ export const TabButton = forwardRef(({
}: TabButtonProps, ref: Ref<HTMLButtonElement>): ReactElement => {
const { t } = useTranslation('tabs');
const dataAttributes = useDataAttributes(rest);
const dataTestId = dataAttributes['data-testid'] ?? 'tabs-tab';
const dataTestId = dataAttributes['data-testid'] ?? 'tab';
const hasRemove = !!onRemove;

return (
<StyledTab $selected={isSelected} data-testid={dataTestId}>
<StyledTab
$selected={isSelected}
data-testid={dataTestId}
onKeyDown={onKeyDown}
>
<StyledButton
type="button"
id={id}
Expand All @@ -140,7 +140,6 @@ export const TabButton = forwardRef(({
data-testid={`${dataTestId}-button`}
tabIndex={isSelected ? undefined : -1}
onClick={onClick}
onKeyDown={onKeyDown}
$removable={hasRemove}
$selected={isSelected}
$global={global}
Expand Down Expand Up @@ -173,6 +172,7 @@ export const TabButton = forwardRef(({
aria-label={t('dismissTab', { label: children })}
iconName='x'
focusable={isSelected}
size="small"
/>
)}
</StyledTab>
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/components/tabs/tab-panel.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ interface TabPanelProps {
}

const StyledDiv = styled.div<{ $isGlobal?: boolean; }>`
background: ${({ $isGlobal, theme }) => !$isGlobal && theme.component['tabs-panel-background-color']};
border: ${({ $isGlobal, theme }) => !$isGlobal && `1px solid ${theme.component['tabs-panel-border-color']}`};
background: ${({ $isGlobal, theme }) => !$isGlobal && theme.component['tab-panel-background-color']};
border: ${({ $isGlobal, theme }) => !$isGlobal && `1px solid ${theme.component['tab-panel-border-color']}`};
border-radius: ${({ $isGlobal }) => !$isGlobal && '0 0 var(--border-radius-2x) var(--border-radius-2x)'};
border-top: none;
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/components/tabs/tabs.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function expectPanelToBeRendered(wrapper: ReactWrapper, tabPanelTestId: string):
}

function getActionButton<W extends ReactWrapper>(wrapper: W, index: number): W {
return getByTestId(wrapper, `tabs-tab-${index}-button`, { htmlNodesOnly: true });
return getByTestId(wrapper, `tab-${index}-button`, { htmlNodesOnly: true });
}

describe('Tabs', () => {
Expand Down Expand Up @@ -87,7 +87,7 @@ describe('Tabs', () => {

getActionButton(wrapper, 2).simulate('click');

expect(getByTestId(wrapper, 'tabs-tab-', { modifier: '^', htmlNodesOnly: true }).length).toBe(1);
expect(getByTestId(wrapper, 'tab-', { modifier: '^', htmlNodesOnly: true }).length).toBe(1);
});

test('tab panel should unmount when another tab is selected', () => {
Expand Down
36 changes: 18 additions & 18 deletions packages/react/src/components/tabs/tabs.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,35 +353,35 @@ exports[`Tabs matches snapshot (global) 1`] = `
>
<div
class="c5"
data-testid="tabs-tab-1"
data-testid="tab-1"
>
<button
aria-controls="uuid1"
aria-selected="true"
class="c6 c7"
data-testid="tabs-tab-1-button"
data-testid="tab-1-button"
id="tab-1"
role="tab"
type="button"
>
<span
class="c8"
data-content="button 1"
data-testid="tabs-tab-1-text"
data-testid="tab-1-text"
>
button 1
</span>
</button>
</div>
<div
class="c9"
data-testid="tabs-tab-2"
data-testid="tab-2"
>
<button
aria-controls="uuid2"
aria-selected="false"
class="c6 c10"
data-testid="tabs-tab-2-button"
data-testid="tab-2-button"
id="tab-2"
role="tab"
tabindex="-1"
Expand All @@ -390,7 +390,7 @@ exports[`Tabs matches snapshot (global) 1`] = `
<span
class="c8"
data-content="button 2"
data-testid="tabs-tab-2-text"
data-testid="tab-2-text"
>
button 2
</span>
Expand Down Expand Up @@ -803,35 +803,35 @@ exports[`Tabs matches snapshot (mobile) 1`] = `
>
<div
class="c5"
data-testid="tabs-tab-1"
data-testid="tab-1"
>
<button
aria-controls="uuid1"
aria-selected="true"
class="c6 c7"
data-testid="tabs-tab-1-button"
data-testid="tab-1-button"
id="tab-1"
role="tab"
type="button"
>
<span
class="c8"
data-content="button 1"
data-testid="tabs-tab-1-text"
data-testid="tab-1-text"
>
button 1
</span>
</button>
</div>
<div
class="c9"
data-testid="tabs-tab-2"
data-testid="tab-2"
>
<button
aria-controls="uuid2"
aria-selected="false"
class="c6 c10"
data-testid="tabs-tab-2-button"
data-testid="tab-2-button"
id="tab-2"
role="tab"
tabindex="-1"
Expand All @@ -840,7 +840,7 @@ exports[`Tabs matches snapshot (mobile) 1`] = `
<span
class="c8"
data-content="button 2"
data-testid="tabs-tab-2-text"
data-testid="tab-2-text"
>
button 2
</span>
Expand Down Expand Up @@ -1253,35 +1253,35 @@ exports[`Tabs matches snapshot 1`] = `
>
<div
class="c5"
data-testid="tabs-tab-1"
data-testid="tab-1"
>
<button
aria-controls="uuid1"
aria-selected="true"
class="c6 c7"
data-testid="tabs-tab-1-button"
data-testid="tab-1-button"
id="tab-1"
role="tab"
type="button"
>
<span
class="c8"
data-content="button 1"
data-testid="tabs-tab-1-text"
data-testid="tab-1-text"
>
button 1
</span>
</button>
</div>
<div
class="c9"
data-testid="tabs-tab-2"
data-testid="tab-2"
>
<button
aria-controls="uuid2"
aria-selected="false"
class="c6 c10"
data-testid="tabs-tab-2-button"
data-testid="tab-2-button"
id="tab-2"
role="tab"
tabindex="-1"
Expand All @@ -1290,7 +1290,7 @@ exports[`Tabs matches snapshot 1`] = `
<span
class="c8"
data-content="button 2"
data-testid="tabs-tab-2-text"
data-testid="tab-2-text"
>
button 2
</span>
Expand Down
Loading

0 comments on commit 575c8a6

Please sign in to comment.