fix(keyboard): fix fireInputEventIfNeeded (#583)

[fergusmcdonald]

What: #583

When a text input with a single character value is selected, typing text
which starts with the same character results in the first character of
the typed text not to be typed.

This occurs as fireInputEventIfNeeded does not fire an input event due
to the new value matching the previous value of the input. The next
character is then typed and the selected text is then overwritten.

This fix changes the fireInputEventIfNeeded condition from no event
being fired when the previous and new values match to an event being
fired in this scenario when the element value has a selection.

Why:

To support the scenario in #583 to match expected events.

How:

The sandbox shows that the scenario outlined in the issue and test
should fire an input event which leads to the selection being
replaced so that the remainder of the text can be typed. This
change adds that input event to be fired and does not impact any
of the existing tests or snapshots.

Checklist:

• Documentation
• Tests
• Typings
• Ready to be merged

[codecov]

Codecov Report

Merging #607 (02037d4) into master (74d191c) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #607   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           47        48    +1     
  Lines          879       882    +3     
  Branches       330       330           
=========================================
+ Hits           879       882    +3     

Impacted Files	Coverage Δ
src/keyboard/shared/fireInputEventIfNeeded.ts	100.00% <100.00%> (ø)
src/utils/edit/hasSelection.ts	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74d191c...02037d4. Read the comment docs.

[fergusmcdonald]

@ph-fritsche - This draft is an attempt at fixing where typing the same character as highlighted into a text field was not firing an input event. This had resulted in the selection persisting and the first character to be typed to be overwritten by the second.

I'm not sure what confidence levels the existing test passing and snapshots not changing gives for a change to made. A review would be greatly appreciated. 🙏

[ph-fritsche]

@fergusmcdonald First of all: Thanks a lot for taking the time to contribute. ❤️

I would like to avoid introducing more workarounds for edge cases when we might be able to achieve the desired results by removing/refactoring code.

There are six references of fireInputEventIfNeeded. I'm not sure they really have a common requirement when calling this function. We might be able to move (some of) the conditions directly into the handlers that require them.

Instead of checking isClickableInput inside fireInputEventIfNeeded, we should only apply the editing behavior described for keypress when the element is editable. Same for isReadonly.
This might be achieved by a isEditable check inside behaviorPlugin.matches.

The comparison with the previous value obviously follows a flawed premise. We should remove it.
We need to verify if the date and time input require this or a similar check. If they do, we should add it there.
We need to add new checks in the behavior for [Backspace] and [Delete] if the cursor is at position 0 respectively length.

This reduces fireInputEventIfNeeded to fireInputEvent and it only applies target.value or target.textContent depending on isContentEditable.