Skip to content

Conversation

tmlayton
Copy link
Contributor

@tmlayton tmlayton commented Jan 8, 2019

WHY are these changes introduced?

step was missing from the input. This caused arrow keys to only step by 1. I'm guessing this also opened the component up other potential under-the-hood and a11y issues.

WHAT is this pull request doing?

Passing the step attribute to the input. The test is added first, which fails. The second commit satisfies the test.

🎩 checklist

Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

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

Passing the step attribute to the input. The test is added first, which fails. The second commit satisfies the test.

❤️

Tested with this Playground code and the step attribute is set on the input as expected, good catch 👍

Click to view collapsed Playground code
import * as React from 'react';
import {Page, Layout, Card, FormLayout, TextField} from '@shopify/polaris';

interface State {
  number: string;
  step: string;
}

export default class Playground extends React.Component<{}, State> {
  state = {
    number: '15',
    step: '5',
  };

  handleChange = (field: string) => (value: string) => {
    this.setState({[field]: value});
  };

  render() {
    return (
      <Page
        title="Playground"
        primaryAction={{content: 'View Examples', url: '/examples'}}
      >
        <Layout>
          <Layout.Section>
            <Card sectioned>
              <FormLayout.Group condensed>
                <TextField
                  label="Step"
                  value={this.state.step}
                  onChange={this.handleChange('step')}
                />
                <TextField
                  step={Number(this.state.step)}
                  type="number"
                  label="Pick a number, any number"
                  value={this.state.number}
                  onChange={this.handleChange('number')}
                />
              </FormLayout.Group>
            </Card>
          </Layout.Section>
        </Layout>
      </Page>
    );
  }
}

@danrosenthal danrosenthal merged commit 9737427 into master Jan 8, 2019
@danrosenthal danrosenthal deleted the fix-step branch January 8, 2019 14:04
@danrosenthal danrosenthal temporarily deployed to production January 8, 2019 15:33 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants