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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix isInputField #4271

Merged
merged 1 commit into from Jan 4, 2018

Conversation

Projects
None yet
3 participants
@iseulde
Member

iseulde commented Jan 3, 2018

Description

Fixes #4268. I accidentally checked for lower case names in nodeName. 馃檲 Adds tests too.

How Has This Been Tested?

Select a block, then select some text in an input field inside the block or in the inspector. The content of the section should be copied, not the whole block.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

@iseulde iseulde requested review from mcsf and aduth Jan 3, 2018

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde

iseulde Jan 3, 2018

Member

Not sure about the tests btw, maybe that's a bit too much. :)

Member

iseulde commented Jan 3, 2018

Not sure about the tests btw, maybe that's a bit too much. :)

@mcsf

mcsf approved these changes Jan 4, 2018

馃殺

Also, I think the tests are fine.

@iseulde iseulde merged commit 38177d6 into master Jan 4, 2018

3 checks passed

codecov/project 39.46% (+0.34%) compared to 2d559e3
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@iseulde iseulde deleted the fix/copy-block-with-input-focus branch Jan 4, 2018

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Jan 4, 2018

Member

What should I expect as the result of isInputField given HTML:

<div contenteditable><span></span></div>

...and called as:

isInputField( document.querySelector( 'div > span' ) );

?

Member

aduth commented Jan 4, 2018

What should I expect as the result of isInputField given HTML:

<div contenteditable><span></span></div>

...and called as:

isInputField( document.querySelector( 'div > span' ) );

?

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde

iseulde Jan 4, 2018

Member
Member

iseulde commented Jan 4, 2018

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Jan 5, 2018

Member

Since it鈥檚 not the input field itself, I guess false?

Yeah, I guess the intention of the question is less "what would it return" (it returns false) and more: should it return false? Or should we consider the span to still be an "input" since it's technically editable?

Certainly an edge case, but one which I expect would be no fun to debug if it were encountered 馃槃

Member

aduth commented Jan 5, 2018

Since it鈥檚 not the input field itself, I guess false?

Yeah, I guess the intention of the question is less "what would it return" (it returns false) and more: should it return false? Or should we consider the span to still be an "input" since it's technically editable?

Certainly an edge case, but one which I expect would be no fun to debug if it were encountered 馃槃

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde

iseulde Jan 8, 2018

Member

Hm, I would assume the person using isInputField is looking for the focusable input field. A span in side a contentEditable area is not an input field? 馃檪

Member

iseulde commented Jan 8, 2018

Hm, I would assume the person using isInputField is looking for the focusable input field. A span in side a contentEditable area is not an input field? 馃檪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment