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

Support string values for capture attribute. #11424

Merged
merged 1 commit into from Nov 12, 2017

Conversation

Projects
None yet
4 participants
@maxschmeling
Contributor

maxschmeling commented Nov 1, 2017

  • Uses HAS_OVERLOADED_BOOLEAN_VALUE instead of HAS_BOOLEAN_VALUE
  • Allows for <input type="file" capture="user" />

Fixes #11419

Support string values for capture attribute.
  * Uses HAS_OVERLOADED_BOOLEAN_VALUE instead of HAS_BOOLEAN_VALUE
  * Allows for <input type="file" capture="user" />
Fixes #11419
@nhunzaker

Thanks for sending this out!

@nhunzaker

This comment has been minimized.

Show comment
Hide comment
@nhunzaker

nhunzaker Nov 12, 2017

Collaborator

Did a couple of tests locally:

it('...', () => {
  var stub = ReactTestUtils.renderIntoDocument(<input capture={false} />);
  var node = ReactDOM.findDOMNode(stub);

  expect(node.getAttribute('capture')).toBe(null)
});

it('...', () => {
  var stub = ReactTestUtils.renderIntoDocument(<input capture={true} />);
  var node = ReactDOM.findDOMNode(stub);

  expect(node.getAttribute('capture')).toBe('')
});

it('...', () => {
  var stub = ReactTestUtils.renderIntoDocument(<input capture="user" />);
  var node = ReactDOM.findDOMNode(stub);

  expect(node.getAttribute('capture')).toBe('user')
});

it('...', () => {
  var stub = ReactTestUtils.renderIntoDocument(<input capture />);
  var node = ReactDOM.findDOMNode(stub);

  expect(node.getAttribute('capture')).toBe('')
});

I think this is pretty safe for a minor.

Collaborator

nhunzaker commented Nov 12, 2017

Did a couple of tests locally:

it('...', () => {
  var stub = ReactTestUtils.renderIntoDocument(<input capture={false} />);
  var node = ReactDOM.findDOMNode(stub);

  expect(node.getAttribute('capture')).toBe(null)
});

it('...', () => {
  var stub = ReactTestUtils.renderIntoDocument(<input capture={true} />);
  var node = ReactDOM.findDOMNode(stub);

  expect(node.getAttribute('capture')).toBe('')
});

it('...', () => {
  var stub = ReactTestUtils.renderIntoDocument(<input capture="user" />);
  var node = ReactDOM.findDOMNode(stub);

  expect(node.getAttribute('capture')).toBe('user')
});

it('...', () => {
  var stub = ReactTestUtils.renderIntoDocument(<input capture />);
  var node = ReactDOM.findDOMNode(stub);

  expect(node.getAttribute('capture')).toBe('')
});

I think this is pretty safe for a minor.

@nhunzaker nhunzaker merged commit 2fe3494 into facebook:master Nov 12, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Nov 13, 2017

Member

I think this is pretty safe for a minor.

Ended up in a patch 😛

We cut releases from master now, so if something should only be merged for a minor, it is best to raise an issue and coordinate. Otherwise we lose the ability to cut quick patch fixes.

But I'm okay treating this one as a patch tbh.

Member

gaearon commented Nov 13, 2017

I think this is pretty safe for a minor.

Ended up in a patch 😛

We cut releases from master now, so if something should only be merged for a minor, it is best to raise an issue and coordinate. Otherwise we lose the ability to cut quick patch fixes.

But I'm okay treating this one as a patch tbh.

@nhunzaker

This comment has been minimized.

Show comment
Hide comment
@nhunzaker

nhunzaker Nov 13, 2017

Collaborator

Hehe. No problem. Sounds good! A patch sounds fine to me too.

Collaborator

nhunzaker commented Nov 13, 2017

Hehe. No problem. Sounds good! A patch sounds fine to me too.

gaearon added a commit that referenced this pull request Dec 1, 2017

Update the attribute table
- the capture attribute changed in #11424
- changes to value/defaultValue handling of functions/Symbols are from #11534, but as per #11734 (comment) this is actually not a new problem so we're okay with it

Ethan-Arrowood added a commit to Ethan-Arrowood/react that referenced this pull request Dec 8, 2017

Support string values for capture attribute. (facebook#11424)
* Uses HAS_OVERLOADED_BOOLEAN_VALUE instead of HAS_BOOLEAN_VALUE
  * Allows for <input type="file" capture="user" />
Fixes facebook#11419
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment