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

Support string values for capture attribute. #11424

Merged
merged 1 commit into from Nov 12, 2017
Merged

Support string values for capture attribute. #11424

merged 1 commit into from Nov 12, 2017

Conversation

maxschmeling
Copy link
Contributor

@maxschmeling maxschmeling commented Nov 1, 2017

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

Fixes #11419

  * Uses HAS_OVERLOADED_BOOLEAN_VALUE instead of HAS_BOOLEAN_VALUE
  * Allows for <input type="file" capture="user" />
Fixes #11419
Copy link
Contributor

@nhunzaker nhunzaker left a comment

Choose a reason for hiding this comment

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

Thanks for sending this out!

@nhunzaker
Copy link
Contributor

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
@gaearon
Copy link
Collaborator

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
Copy link
Contributor

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

gaearon added a commit that referenced this pull request Dec 1, 2017
- 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 pushed a commit to Ethan-Arrowood/react that referenced this pull request Dec 8, 2017
* 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants