Skip to content
Open
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
323 changes: 82 additions & 241 deletions package-lock.json
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I assume we can just revert these changes and things will still work, I know the package lock has been updated quite a bit since this PR was opened.

Large diffs are not rendered by default.

32 changes: 19 additions & 13 deletions src/Stepper/Stepper.jsx → src/Stepper/Stepper.tsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,34 @@
import React from 'react';
import PropTypes from 'prop-types';
import StepperStep from './StepperStep';
import StepperHeader from './StepperHeader';
import StepperActionRow from './StepperActionRow';
import { StepperContextProvider } from './StepperContext';

function Stepper({ children, activeKey }) {
return (
<StepperContextProvider activeKey={activeKey}>
{children}
</StepperContextProvider>
);
}

Stepper.propTypes = {
export interface StepperProps {
/**
* Specifies the content of the `Stepper`.
*/
children: PropTypes.node.isRequired,
children: React.ReactNode;
/**
* The eventKey of the step to display.
*/
activeKey: PropTypes.string.isRequired,
};
activeKey: string;
}

interface StepperComponent {
(props: StepperProps): JSX.Element;
Step: typeof StepperStep;
Header: typeof StepperHeader;
ActionRow: typeof StepperActionRow;
}

const Stepper: StepperComponent = function Stepper({ children, activeKey }: StepperProps) {
return (
<StepperContextProvider activeKey={activeKey}>
{children}
</StepperContextProvider>
);
} as StepperComponent;

Stepper.Step = StepperStep;
Stepper.Header = StepperHeader;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,31 @@
import React, { useContext } from 'react';
import PropTypes from 'prop-types';
import { StepperContext } from './StepperContext';
import ActionRow from '../ActionRow';

export interface StepperActionRowProps {
/** Specifies the content of the `ActionRow`. */
children: React.ReactNode;
/**
* An identifier of the `ActionRow`. When `activeKey` on the
* `Stepper` equals to the `eventKey`, the `ActionRow` will be displayed.
*/
eventKey: string;
/** Specifies the base element */
as?: React.ElementType;
[key: string]: any; // For additional props passed through
Comment on lines +14 to +15
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This came up in the ModalCloseButton PR, using ComponentWithAsProp here instead of just allowing all additional props through unchecked would be better. See #4134 (comment)

}

interface StepperActionRowComponent {
(props: StepperActionRowProps): JSX.Element | null;
Spacer: typeof ActionRow.Spacer;
}

function StepperActionRow({
as,
as = ActionRow,
children,
eventKey,
...props
}) {
}: StepperActionRowProps) {
const { activeKey } = useContext(StepperContext);
const isActive = activeKey === eventKey;

Expand All @@ -19,22 +36,6 @@ function StepperActionRow({
return React.createElement(as, props, children);
}

StepperActionRow.propTypes = {
/** Specifies the content of the `ActionRow`. */
children: PropTypes.node.isRequired,
/**
* An identifier of the `ActionRow`. When `activeKey` on the
* `Stepper` equals to the `eventKey`, the `ActionRow` will be displayed.
*/
eventKey: PropTypes.string.isRequired,
/** Specifies the base element */
as: PropTypes.elementType,
};

StepperActionRow.defaultProps = {
as: ActionRow,
};

StepperActionRow.Spacer = ActionRow.Spacer;
(StepperActionRow as StepperActionRowComponent).Spacer = ActionRow.Spacer;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same thing here as https://github.com/openedx/paragon/pull/4137/changes#r3174512641, following the pattern from Stepper would be better

const Stepper: StepperComponent = function Stepper({ children, activeKey }: StepperProps) {


export default StepperActionRow;
58 changes: 41 additions & 17 deletions src/Stepper/StepperContext.jsx → src/Stepper/StepperContext.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,38 @@
import React, {
useCallback, useEffect, useReducer, useState,
} from 'react';
import PropTypes from 'prop-types';

export const StepperContext = React.createContext({
export interface Step {
eventKey: string;
title: string;
description?: string;
hasError?: boolean;
index?: number;
onClick?: () => void;
}

export interface StepperContextValue {
activeKey: string;
registerStep: (step: Step) => void;
steps: Step[];
removeStep: (eventKey: string) => void;
getIsViewed: (index: number) => boolean;
}

export const StepperContext = React.createContext<StepperContextValue>({
activeKey: '',
registerStep: () => {},
steps: [],
removeStep: () => {},
getIsViewed: () => false,
});

const stepsReducer = (stepsState, action) => {
let newStepsState = [];
type StepsAction =
| { type: 'register'; step: Step }
| { type: 'remove'; eventKey: string };

const stepsReducer = (stepsState: Step[], action: StepsAction): Step[] => {
let newStepsState: Step[] = [];
switch (action.type) {
case 'remove':
return stepsState.filter(step => step.eventKey !== action.eventKey);
Expand All @@ -27,22 +51,29 @@ const stepsReducer = (stepsState, action) => {
}

// If using the index prop
if (stepsState.some(step => step.index)) {
if (stepsState.some(step => step.index !== undefined)) {
return newStepsState.sort((a, b) => (
a.index > b.index ? 1 : -1
(a.index || 0) > (b.index || 0) ? 1 : -1
));
}
return newStepsState;
}
};

export function StepperContextProvider({ children, activeKey }) {
export interface StepperContextProviderProps {
/** Specifies the content of the `ContextProvider`. */
children: React.ReactNode;
/** Specifies the current step of the `Stepper`. */
activeKey: string;
}

export function StepperContextProvider({ children, activeKey }: StepperContextProviderProps) {
const [steps, dispatch] = useReducer(stepsReducer, []);
const [currentBoundary, setCurrentBoundary] = useState(0);
const registerStep = useCallback((step) => dispatch({ step, type: 'register' }), []);
const removeStep = useCallback((eventKey) => dispatch({ eventKey, type: 'remove' }), []);
const registerStep = useCallback((step: Step) => dispatch({ step, type: 'register' }), []);
const removeStep = useCallback((eventKey: string) => dispatch({ eventKey, type: 'remove' }), []);

const getIsViewed = (index) => index <= currentBoundary;
const getIsViewed = (index: number) => index <= currentBoundary;

useEffect(() => {
const activeIndex = steps.findIndex(step => step.eventKey === activeKey);
Expand All @@ -64,11 +95,4 @@ export function StepperContextProvider({ children, activeKey }) {
);
}

StepperContextProvider.propTypes = {
/** Specifies the content of the `ContextProvider`. */
children: PropTypes.node.isRequired,
/** Specifies the current step of the `Stepper`. */
activeKey: PropTypes.node.isRequired,
};

export { stepsReducer };
89 changes: 47 additions & 42 deletions src/Stepper/StepperHeader.jsx → src/Stepper/StepperHeader.tsx
Original file line number Diff line number Diff line change
@@ -1,50 +1,86 @@
import React, { useContext } from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import StepperHeaderStep from './StepperHeaderStep';
import { StepperContext } from './StepperContext';
import { StepperContext, Step } from './StepperContext';
import useWindowSize from '../hooks/useWindowSizeHook';
import breakpoints, { Size } from '../utils/breakpoints';

function StepListSeparator() {
return <li aria-hidden="true" className="pgn__stepper-header-line" />;
}

function StepList({ steps, activeKey }) {
export interface StepListProps {
steps: Step[];
activeKey: string;
}

function StepList({ steps, activeKey }: StepListProps) {
return (
<ul className="pgn__stepper-header-step-list">
{steps.map(({ label, ...stepProps }, index) => (
{steps.map(({ title: label, ...stepProps }, index) => (
<React.Fragment key={stepProps.eventKey}>
{index !== 0 && <StepListSeparator />}
<StepperHeaderStep
{...stepProps}
title={label}
index={index}
isActive={activeKey === stepProps.eventKey}
>
{label}
</StepperHeaderStep>
/>
</React.Fragment>
))}
</ul>
);
}
Comment on lines +17 to 33
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like label has never been used and has always been undefined here (going all the way back to when the component was first added in #690)

With that in mind, we can simplify this to

Suggested change
function StepList({ steps, activeKey }: StepListProps) {
return (
<ul className="pgn__stepper-header-step-list">
{steps.map(({ label, ...stepProps }, index) => (
{steps.map(({ title: label, ...stepProps }, index) => (
<React.Fragment key={stepProps.eventKey}>
{index !== 0 && <StepListSeparator />}
<StepperHeaderStep
{...stepProps}
title={label}
index={index}
isActive={activeKey === stepProps.eventKey}
>
{label}
</StepperHeaderStep>
/>
</React.Fragment>
))}
</ul>
);
}
function StepList({ steps, activeKey }: StepListProps) {
return (
<ul className="pgn__stepper-header-step-list">
{steps.map((step, index) => (
<React.Fragment key={step.eventKey}>
{index !== 0 && <StepListSeparator />}
<StepperHeaderStep
{...step}
index={index}
isActive={activeKey === step.eventKey}
/>
</React.Fragment>
))}
</ul>
);
}


const PageCount = ({ activeStepIndex, totalSteps }) => `Step ${activeStepIndex + 1} of ${totalSteps}`;
export interface PageCountProps {
activeStepIndex: number;
totalSteps: number;
}

function PageCount({ activeStepIndex, totalSteps }: PageCountProps) {
return <>Step {activeStepIndex + 1} of {totalSteps}</>;
}

export interface StepperHeaderProps {
/** Specifies class name to append to the base element. */
className?: string | null;
/** A component that receives `activeStepIndex` and `totalSteps` props to display them. */
PageCountComponent?: React.ComponentType<PageCountProps>;
/** The max width in which the compact view of the header will switch to display the step number that is
* currently in progress. Options include 'xs', 'sm', 'md', 'lg', 'xl', and 'xxl'.
*/
compactWidth?: 'xs' | 'sm' | 'md' | 'lg' | 'xl' | 'xxl';
}

interface StepperHeaderComponent {
(props: StepperHeaderProps): JSX.Element | null;
Step: typeof StepperHeaderStep;
}

function StepperHeader({ className, PageCountComponent, compactWidth }) {
function StepperHeader({
className = null,
PageCountComponent = PageCount,
compactWidth = 'sm',
}: StepperHeaderProps) {
const { steps, activeKey } = useContext(StepperContext);
const windowDimensions = useWindowSize();
const size = Size[compactWidth] || 'small';
const breakpointWidth = breakpoints[size].maxWidth || Infinity;
const isCompactView = windowDimensions.width < breakpointWidth;
const isCompactView = (windowDimensions.width ?? 0) < breakpointWidth;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is changing the behavior slightly. When windowDimensions.width is undefined, windowDimensions.width < breakpointWidth is false, but (windowDimensions.width ?? 0) < breakpointWidth is true.

We can replicate the original behavior with

Suggested change
const isCompactView = (windowDimensions.width ?? 0) < breakpointWidth;
const isCompactView = windowDimensions.width !== undefined && windowDimensions.width < breakpointWidth;


if (isCompactView) {
const activeStepIndex = steps.findIndex(step => step.eventKey === activeKey);
const activeStep = steps[activeStepIndex];

if (!activeStep) {
return null;
}

return (
<div className={classNames('pgn__stepper-header', className)}>
<StepperHeaderStep
{...activeStep}
title={activeStep.title}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's not clear to me why we need to specify this here. We're spreading the activeStep props which include title, and index={activeStepIndex} ensures we override the index from the spread with the index in our steps array (as opposed to the index prop on the step itself), but setting title here seems to just be redundant as the title from spread will be the same as activeStep.title

index={activeStepIndex}
isActive
/>
Expand All @@ -67,37 +103,6 @@ function StepperHeader({ className, PageCountComponent, compactWidth }) {
);
}

StepperHeader.propTypes = {
/** Specifies class name to append to the base element. */
className: PropTypes.string,
/** A component that receives `activeStepIndex` and `totalSteps` props to display them. */
PageCountComponent: PropTypes.elementType,
/** The max width in which the compact view of the header will switch to display the step number that is
* currently in progress. Options include 'xs', 'sm', 'md', 'lg', 'xl', and 'xxl'.
*/
compactWidth: PropTypes.oneOf(['xs', 'sm', 'md', 'lg', 'xl', 'xxl']),
};

StepperHeader.defaultProps = {
className: null,
PageCountComponent: PageCount,
compactWidth: 'sm',
};

StepList.propTypes = {
steps: PropTypes.arrayOf(PropTypes.shape({
eventKey: PropTypes.string,
title: PropTypes.string,
description: PropTypes.string,
hasError: PropTypes.bool,
})),
activeKey: PropTypes.string.isRequired,
};

StepList.defaultProps = {
steps: [],
};

StepperHeader.Step = StepperHeaderStep;
(StepperHeader as StepperHeaderComponent).Step = StepperHeaderStep;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This cast feels like it could cause some problems. In this PR's Stepper.tsx this is handled properly with

const Stepper: StepperComponent = function Stepper({ children, activeKey }: StepperProps) {

it'd be great to follow that pattern here too.


export default StepperHeader;
Original file line number Diff line number Diff line change
@@ -1,20 +1,34 @@
import React, { useContext } from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';

import { Check, Error } from '../../icons';
import { StepperContext } from './StepperContext';
import Icon from '../Icon';
import Bubble from '../Bubble';

export interface StepperHeaderStepProps {
/** A number that will be display in the icon of the `HeaderStep`. */
index: number;
/** A text of the `HeaderStep`. */
title: string;
/** Specifies that this `HeaderStep` is active. */
isActive?: boolean;
/** Informs user if this `Step` has errors. */
hasError?: boolean;
/** A text under the `title`. */
description?: string;
/** Callback fired when element gets clicked. */
onClick?: () => void;
}

function StepperHeaderStep({
title,
isActive,
hasError,
isActive = false,
hasError = false,
description,
index,
onClick,
}) {
}: StepperHeaderStepProps) {
const { getIsViewed } = useContext(StepperContext);
const isComplete = getIsViewed(index + 1);
const isViewed = getIsViewed(index);
Expand Down Expand Up @@ -71,26 +85,4 @@ function StepperHeaderStep({
);
}

StepperHeaderStep.propTypes = {
/** A number that will be display in the icon of the `HeaderStep`. */
index: PropTypes.number.isRequired,
/** A text of the `HeaderStep`. */
title: PropTypes.string.isRequired,
/** Specifies that this `HeaderStep` is active. */
isActive: PropTypes.bool,
/** Informs user if this `Step` has errors. */
hasError: PropTypes.bool,
/** A text under the `title`. */
description: PropTypes.string,
/** Callback fired when element gets clicked. */
onClick: PropTypes.func,
};

StepperHeaderStep.defaultProps = {
isActive: false,
hasError: false,
description: undefined,
onClick: undefined,
};

export default StepperHeaderStep;
Loading