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

add Steps component #202

Merged
merged 6 commits into from
Dec 14, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions src-docs/src/services/routes/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ import { SideNavExample }
import { SpacerExample }
from '../../views/spacer/spacer_example';

import { StepsExample }
from '../../views/steps/steps_example';

import { TableExample }
from '../../views/table/table_example';

Expand Down Expand Up @@ -200,6 +203,7 @@ const components = [
ProgressExample,
SideNavExample,
SpacerExample,
StepsExample,
TableExample,
TabsExample,
TextExample,
Expand Down
59 changes: 59 additions & 0 deletions src-docs/src/views/steps/steps.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import React from 'react';

import {
EuiCode,
EuiSpacer,
EuiSteps,
EuiText,
} from '../../../../src/components';

const firstSetOfSteps = [
{
title: 'step 1',
children: (<p>{'Do this first'}</p>)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same nit about parens here and elsewhere in the file.

},
{
title: 'step 2',
children: (<p>{'Then this'}</p>)
},
{
title: 'step 3',
children: (<p>{'And finally, do this'}</p>)
},
];

const nextSetOfSteps = [
{
title: 'good step',
children: (<p>{'Do this first'}</p>)
},
{
title: 'better step',
children: (<p>{'Then this'}</p>)
},
{
title: 'best step',
children: (<p>{'And finally, do this'}</p>)
},
];

export default () => (
<div>
<EuiSteps
steps={firstSetOfSteps}
/>

<EuiText>
<EuiSpacer size="m"/>
<p>
Set <EuiCode>offset</EuiCode> to continue step numbering after any type of break in the content
</p>
<EuiSpacer size="m"/>
</EuiText>

<EuiSteps
offset={firstSetOfSteps.length}
steps={nextSetOfSteps}
/>
</div>
);
31 changes: 31 additions & 0 deletions src-docs/src/views/steps/steps_example.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React from 'react';

import { renderToHtml } from '../../services';

import {
GuideSectionTypes,
} from '../../components';

import Steps from './steps';
const stepsSource = require('!!raw-loader!./steps');
const stepsHtml = renderToHtml(Steps);

export const StepsExample = {
title: 'Steps',
sections: [{
title: 'Steps',
source: [{
type: GuideSectionTypes.JS,
code: stepsSource,
}, {
type: GuideSectionTypes.HTML,
code: stepsHtml,
}],
text: (
<p>
Numbered steps
</p>
),
demo: <Steps />,
}],
};
4 changes: 4 additions & 0 deletions src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ export {
EuiSpacer,
} from './spacer';

export {
EuiSteps,
} from './steps';

export {
EuiTable,
EuiTableBody,
Expand Down
1 change: 1 addition & 0 deletions src/components/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
@import 'progress/index';
@import 'side_nav/index';
@import 'spacer/index';
@import 'steps/index';
@import 'table/index';
@import 'tabs/index';
@import 'title/index';
Expand Down
145 changes: 145 additions & 0 deletions src/components/steps/__snapshots__/steps.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`EuiSteps renders steps 1`] = `
<div
aria-label="aria-label"
class="euiSteps testClass1 testClass2"
data-test-subj="test subject string"
>
<div>
<div>
<div
class="euiStepNumber"
>
1
</div>
<h3
class="euiStepTitle"
>
first title
</h3>
</div>
<div
class="euiStep"
>
<p>
Do this first
</p>
</div>
</div>
<div>
<div>
<div
class="euiStepNumber"
>
2
</div>
<h3
class="euiStepTitle"
>
second title
</h3>
</div>
<div
class="euiStep"
>
<p>
Then this
</p>
</div>
</div>
<div>
<div>
<div
class="euiStepNumber"
>
3
</div>
<h3
class="euiStepTitle"
>
third title
</h3>
</div>
<div
class="euiStep"
>
<p>
And finally, do this
</p>
</div>
</div>
</div>
`;

exports[`EuiSteps renders steps with offset 1`] = `
<div
aria-label="aria-label"
class="euiSteps testClass1 testClass2"
data-test-subj="test subject string"
>
<div>
<div>
<div
class="euiStepNumber"
>
10
</div>
<h3
class="euiStepTitle"
>
first title
</h3>
</div>
<div
class="euiStep"
>
<p>
Do this first
</p>
</div>
</div>
<div>
<div>
<div
class="euiStepNumber"
>
11
</div>
<h3
class="euiStepTitle"
>
second title
</h3>
</div>
<div
class="euiStep"
>
<p>
Then this
</p>
</div>
</div>
<div>
<div>
<div
class="euiStepNumber"
>
12
</div>
<h3
class="euiStepTitle"
>
third title
</h3>
</div>
<div
class="euiStep"
>
<p>
And finally, do this
</p>
</div>
</div>
</div>
`;
1 change: 1 addition & 0 deletions src/components/steps/_index.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@import 'steps';
27 changes: 27 additions & 0 deletions src/components/steps/_steps.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
.euiSteps {

}

.euiStepNumber {
width: 32px;
height: 32px;
display: inline-block;
line-height: 32px;
text-align: center;
color: #FFF;
border-radius: 100%;
background-color: #0079a5;
}

.euiStepTitle {
display: inline-block;
margin-left: 16px;
}

.euiStep {
border-left: medium solid #D9D9D9;
margin-top: 8px;
margin-bottom: 8px;
margin-left: 16px;
padding-left: 24px;
}
3 changes: 3 additions & 0 deletions src/components/steps/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export {
EuiSteps,
} from './steps';
29 changes: 29 additions & 0 deletions src/components/steps/step.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import React from 'react';
import PropTypes from 'prop-types';

export function EuiStep({ children, step, title }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add className and ...rest to this destructuring assignment?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a snapshot test for this component too?

return (
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

And can we assign them as props to this div?

Copy link
Contributor

Choose a reason for hiding this comment

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

As well as add a standard component class of euiStep?


<div>
<div className="euiStepNumber">
{step}
</div>
<h3 className="euiStepTitle">
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we can hardcode the h3 here. For accessibility purposes, headings should always be sequential, so if there is an <h3> on the page then there must also be at least one <h2> and only one <h1>.

I think our best option would be to allow the consumer to specify which element gets used for the heading, and to default to <p> if none is specified. This could be specified as a headingElement prop on KuiSteps, since I think we'd want to use the same heading element for each KuiStep within a KuiSteps instance.

We'd probably also want to put the step number inside of the heading element too, if possible, so that screen readers will read the number and heading text together.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, can we reuse the EuiTitle component here?

    <EuiTitle>
      <headingElement>{title}</headingElement>
    </EuiTitle>

{title}
</h3>
</div>

<div className="euiStep">
Copy link
Contributor

Choose a reason for hiding this comment

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

@nreese Can you actually put the .euiStep className on the entire wrapping div and rename this one to be euiStepContent? This way I can know which EuiStep is the last one in the EuiSteps container.

{children}
</div>

</div>
);
}

EuiStep.propTypes = {
children: PropTypes.node.isRequired,
step: PropTypes.number.isRequired,
title: PropTypes.string.isRequired,
};
46 changes: 46 additions & 0 deletions src/components/steps/steps.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import { EuiStep } from './step';

function renderSteps(steps, offset = 0) {
return steps.map((step, index) => (
<EuiStep
className="kuiVerticalRhythm"
Copy link
Contributor

Choose a reason for hiding this comment

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

kui? What is this kui you speak of? :)

Can we also change this logic to allow the consumer to specify className and other arbitrary props on the step object?

function renderSteps(steps, firstStepNumber) {
  return steps.map((step, index) => {
    const {
      className,
      children,
      title,
      ...rest
    } = step;
  
    return (
      <EuiStep
        className={className}
        key={index}
        step={firstStepNumber + index}
        title={title}
        {...rest}
      >
        {children}
      </EuiStep>
    );
  ));
}

key={index}
step={offset + index + 1}
title={step.title}
>
{step.children}
</EuiStep>
));
}

export const EuiSteps = ({
className,
offset,
steps,
...rest,
}) => {
const classes = classNames('euiSteps', className);

return (
<div
className={classes}
{...rest}
>
{renderSteps(steps, offset)}
</div>
);
};

const stepPropType = PropTypes.shape({
title: PropTypes.string.isRequired,
children: PropTypes.node
});

EuiSteps.propTypes = {
className: PropTypes.string,
offset: PropTypes.number,
Copy link
Contributor

Choose a reason for hiding this comment

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

I had to read the docs to understand the role of this prop, but I think a clearer name would help. Maybe something like firstStepNumber?

Copy link
Contributor

Choose a reason for hiding this comment

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

And can we set defaultProps so this defaults to 1, and remove the default on line 6?

steps: PropTypes.arrayOf(stepPropType).isRequired,
};
Loading