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

Web: Modify components, define new types for yaml parse/stringify endpoints #40974

Merged
merged 8 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -84,7 +84,6 @@ const Wrapper = styled.div`
`;

const WrapperBackground = styled.div`
background: ${props => props.theme.colors.levels.sunken};
border-radius: 200px;
width: 100%;
height: ${props => props.theme.space[8]}px;
Expand Down
31 changes: 25 additions & 6 deletions web/packages/design/src/SlideTabs/SlideTabs.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,49 +16,68 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import React from 'react';
import React, { useState } from 'react';

import SlideTabs from './SlideTabs';
import { SlideTabs } from './SlideTabs';

export default {
title: 'Design/SlideTabs',
};

export const ThreeTabs = () => {
const [activeIndex, setActiveIndex] = useState(0);
return (
<SlideTabs
tabs={['aws', 'automatically', 'manually']}
onChange={() => {}}
onChange={setActiveIndex}
activeIndex={activeIndex}
/>
);
};

export const FiveTabs = () => {
const [activeIndex, setActiveIndex] = useState(0);
return (
<SlideTabs
tabs={['step1', 'step2', 'step3', 'step4', 'step5']}
onChange={() => {}}
onChange={setActiveIndex}
activeIndex={activeIndex}
/>
);
};

export const Round = () => {
const [activeIndex, setActiveIndex] = useState(0);
return (
<SlideTabs
appearance="round"
tabs={['step1', 'step2', 'step3', 'step4', 'step5']}
onChange={() => {}}
onChange={setActiveIndex}
activeIndex={activeIndex}
/>
);
};

export const Medium = () => {
const [activeIndex, setActiveIndex] = useState(0);
return (
<SlideTabs
tabs={['step1', 'step2', 'step3', 'step4', 'step5']}
size="medium"
appearance="round"
onChange={() => {}}
onChange={setActiveIndex}
activeIndex={activeIndex}
/>
);
};

export const LoadingTab = () => {
return (
<SlideTabs
tabs={['aws', 'automatically', 'manually']}
onChange={() => null}
activeIndex={1}
isProcessing={true}
/>
);
};
93 changes: 27 additions & 66 deletions web/packages/design/src/SlideTabs/SlideTabs.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,20 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import React from 'react';
import React, { useState } from 'react';
import { screen } from '@testing-library/react';

import { render, fireEvent } from 'design/utils/testing';
import { render, userEvent } from 'design/utils/testing';

import SlideTabs from './SlideTabs';
import { SlideTabs } from './SlideTabs';

describe('design/SlideTabs', () => {
it('renders the supplied number of tabs(3)', () => {
render(
<SlideTabs
tabs={['aws', 'automatically', 'manually']}
onChange={() => {}}
activeIndex={0}
/>
);

Expand All @@ -44,6 +45,7 @@ describe('design/SlideTabs', () => {
<SlideTabs
tabs={['aws', 'automatically', 'manually', 'apple', 'purple']}
onChange={() => {}}
activeIndex={0}
/>
);

Expand All @@ -62,79 +64,38 @@ describe('design/SlideTabs', () => {
name="pineapple"
tabs={['aws', 'automatically', 'manually']}
onChange={() => {}}
activeIndex={0}
/>
);

// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
expect(container.querySelectorAll('input[name=pineapple]')).toHaveLength(3);
});

it('calls the onChange handler when the tab is changed', () => {
const cb = jest.fn();
render(
<SlideTabs onChange={cb} tabs={['aws', 'automatically', 'manually']} />
);
fireEvent.click(screen.getByText('manually'));

// The reason there are two calls to the callback is because when the
// component is initially rendered it selects the first tab which is in
// index 0 and calls the callback as such.
expect(cb).toHaveBeenNthCalledWith(1, 0);
expect(cb).toHaveBeenNthCalledWith(2, 2);
});

it('supports a square xlarge appearance (default)', () => {
const { container } = render(
<SlideTabs
tabs={['aws', 'automatically', 'manually']}
onChange={() => {}}
/>
);
expect(container).toMatchSnapshot();
});

it('supports a round xlarge appearance', () => {
const { container } = render(
<SlideTabs
appearance="round"
tabs={['aws', 'automatically', 'manually']}
onChange={() => {}}
/>
);
expect(container).toMatchSnapshot();
});
test('onChange highlights the tab clicked', async () => {
render(<Component />);

it('supports a square medium size', () => {
const { container } = render(
<SlideTabs
size="medium"
tabs={['aws', 'automatically', 'manually']}
onChange={() => {}}
/>
);
expect(container).toMatchSnapshot();
});
// First tab is selected by default.
expect(screen.getByRole('tab', { name: 'first' })).toHaveClass('selected');

it('supports a round medium size', () => {
const { container } = render(
<SlideTabs
size="medium"
appearance="round"
tabs={['aws', 'automatically', 'manually']}
onChange={() => {}}
/>
);
expect(container).toMatchSnapshot();
});
// Select the second tab.
await userEvent.click(screen.getByText('second'));
expect(screen.getByRole('tab', { name: 'second' })).toHaveClass('selected');

it('supports passing in a selected index', () => {
const { container } = render(
<SlideTabs
initialSelected={1}
tabs={['aws', 'automatically', 'manually']}
onChange={() => {}}
/>
expect(screen.getByRole('tab', { name: 'first' })).not.toHaveClass(
'selected'
);
expect(container).toMatchSnapshot();
});
});

const Component = () => {
const [activeIndex, setActiveIndex] = useState(0);

return (
<SlideTabs
onChange={setActiveIndex}
tabs={['first', 'second']}
activeIndex={activeIndex}
/>
);
};
84 changes: 59 additions & 25 deletions web/packages/design/src/SlideTabs/SlideTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,40 +16,44 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import React, { useEffect, useState } from 'react';
import React from 'react';
import styled from 'styled-components';

function SlideTabs({
import { Indicator, Text } from '..';

export function SlideTabs({
appearance = 'square',
initialSelected = 0,
activeIndex = 0,
name = 'slide-tab',
onChange,
size = 'xlarge',
tabs,
isProcessing = false,
}: props) {
const [activeIndex, setActiveIndex] = useState(initialSelected);

useEffect(() => {
onChange(activeIndex);
}, [activeIndex]);

return (
<Wrapper>
<TabNav role="tablist" appearance={appearance} size={size}>
{tabs.map((tabData, tabIndex) => {
const tabDataType = typeof tabData === 'string';
const tabName = tabDataType ? tabData : tabData.name;
const tabContent = tabDataType ? tabData : tabData.component;
{tabs.map((tabName, tabIndex) => {
const selected = tabIndex === activeIndex;
return (
<TabLabel
role="tab"
htmlFor={`${name}-${tabName}`}
onClick={() => setActiveIndex(tabIndex)}
onClick={e => {
e.preventDefault();
Copy link
Member

Choose a reason for hiding this comment

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

Is this e.preventDefault() necessary? From what I see, this is just a <label>, so there's no extra behavior on click. Pointer events are turned off when it's processing too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i was confused by it to, but without the e.preventDefault, onChange gets called twice for some reason

onChange(tabIndex);
}}
itemCount={tabs.length}
key={`${tabName}-${tabIndex}`}
className={tabIndex === activeIndex && 'selected'}
processing={isProcessing}
>
{tabContent}
<Box>
{selected && isProcessing && (
<Spinner delay="none" size="25px" />
Copy link
Member

Choose a reason for hiding this comment

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

Because of the absolute positioning, the spinner will get rendered outside of the given tab when the browser width is narrow enough.

Though I understand that this will be used only in a single place, so I guess it's good enough for now.

I suppose the issue was that you didn't want the text of the tab to jump when the spinner gets shown? I wonder if a horizontal progress bar would work here. Connect has one in LinearProgress.tsx, but we don't have stories for it unfortunately. I see that there's a story for "Animated Progress Bar", but this might be too big idk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose the issue was that you didn't want the text of the tab to jump when the spinner gets shown?

yes

i'll add a note on this component to look into horizontal progress bar

)}
<Text ml={2}>{tabName}</Text>
</Box>
<TabInput type="radio" name={name} id={`${name}-${tabName}`} />
</TabLabel>
);
Expand All @@ -66,19 +70,39 @@ function SlideTabs({
}

type props = {
// The style to render the selector in.
/**
* The style to render the selector in.
*/
appearance?: 'square' | 'round';
// The index that you'd like to select on the initial render.
initialSelected?: number;
// The name you'd like to use for the form if using multiple tabs on the page.
// Default: "slide-tab"
/**
* The index that you'd like to select on the initial render.
*/
activeIndex: number;
Copy link
Member

Choose a reason for hiding this comment

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

Good change. 👍 I mean controlling the current tab from outside of the component.

Could you update the comment and turn all comments into JSDocs?

/**
* The name you'd like to use for the form if using multiple tabs on the page.
* Default: "slide-tab"
*/
name?: string;
// To be notified when the selected tab changes supply it with this fn.
/**
* To be notified when the selected tab changes supply it with this fn.
*/
onChange: (selectedTab: number) => void;
// The size to render the selector in.
/**
* The size to render the selector in.
*/
size?: 'xlarge' | 'medium';
// A list of tab names that you'd like displayed in the list of tabs.
tabs: string[] | TabComponent[];
/**
* A list of tab names that you'd like displayed in the list of tabs.
*/
tabs: string[];
/**
* If true, renders a spinner and disables clicking on the tabs.
*
* Currently, a spinner is used in absolute positioning which can render
* outside of the given tab when browser width is narrow enough.
* Look into horizontal progress bar (connect has one in LinearProgress.tsx)
*/
isProcessing?: boolean;
};

export type TabComponent = {
Expand All @@ -97,6 +121,8 @@ const TabLabel = styled.label`
padding: 10px;
width: ${props => 100 / props.itemCount}%;
z-index: 1; /* Ensures that the label is above the background slider. */
opacity: ${p => (p.processing ? 0.5 : 1)};
pointer-events: ${p => (p.processing ? 'none' : 'auto')};
`;

const TabInput = styled.input`
Expand Down Expand Up @@ -131,4 +157,12 @@ const TabNav = styled.nav`
}
`;

export default SlideTabs;
const Spinner = styled(Indicator)`
color: ${p => p.theme.colors.levels.deep};
position: absolute;
left: -${p => p.theme.space[4]}px;
`;

const Box = styled.div`
position: relative;
`;
Loading
Loading