ollelauribostrom / rebus

🌟 👣 Take your first steps as an open source contributor
MIT License
551 stars 844 forks source link

Backspace for Multi-Word Rebus should delete all characters #263

Open cbaron3 opened 4 years ago

cbaron3 commented 4 years ago

Expected Behavior

When typing a solution to a rebus with multiple words, holding the backspace key down should delete all letters until the start of the first word

Actual Behavior

As an example, see https://ollelauribostrom.github.io/rebus/?rebus=15

When solving this rebus, if you hold down backspace while entering in characters in the second word (second row), the cursor stops at the start of the second word. To delete the characters from the first word, you have to either use your mouse to select a box in the first word or SHIFT+TAB

I think it would be useful if you could hold down the backspace button to delete characters in all words, not just the currently selected word

Steps to Reproduce the Problem

Step 1. Find a rebus with multiple words (multiple rows) for example https://ollelauribostrom.github.io/rebus/?rebus=15 Step 2. Start typing characters in any word (row) besides the first one Step 3. Hold the backspace button, cursor will stop at the beginning of the word instead of beginning of the rebus

Specifications

cbaron3 commented 4 years ago

If this is fix-worthy, I would like to work on this issue.

anujgarg204 commented 4 years ago

I did it except I am still not good at writing a test case. See if you can help me with that a bit.

cbaron3 commented 4 years ago

Awesome

For the test case, maybe take a look at the following existing tests cases here: https://github.com/ollelauribostrom/rebus/blob/master/tests/components.spec.js#L101-124

You could create a similar test with the input word: being something that contains more than one word for example word: my test

You could focus on the last index of inputs and create enough backspace events such that you move to the next line. This would result in the active element in the document being the last index of inputs minus the amount of backspace events. Just make sure the amount of backspace events are greater then the length of the last word

anujgarg204 commented 4 years ago

Although the test case is passing locally on my machine, coveralls is still failing.

Here's the code I wrote: https://github.com/ollelauribostrom/rebus/blob/master/src/js/components/Word.js#L38

if (key === 'Backspace' || keyCode === 8) { const input = target.value; const prevChild = target.previousElementSibling; if (prevChild !== null && input === '') { prevChild.focus(); e.preventDefault(); } else if (target != null && target.parentElement && input === '') { const { previousSibling } = target.parentElement.previousSibling; if (previousSibling !== null) { const lastChildOfParent = previousSibling.lastElementChild; if (lastChildOfParent) { lastChildOfParent.focus(); e.preventDefault(); } } } }

And the test case: it('clears everything if you hold backspace', () => { const onInput = jest.fn(); const props = { ...getMockState(), word: 'one word', wordIndex: 0, charInput: onInput }; const root = document.createElement('div'); render(Word(props), root); const inputs = root.querySelectorAll('input'); const mockEvent = new Event('keydown'); mockEvent.key = 'Backspace'; inputs[7].focus(); inputs[7].dispatchEvent(mockEvent); inputs[6].dispatchEvent(mockEvent); inputs[5].dispatchEvent(mockEvent); inputs[4].dispatchEvent(mockEvent); inputs[3].dispatchEvent(mockEvent); expect(inputs[2] === document.activeElement).toEqual(true); });

ishiprak commented 2 years ago

Hi, Please kindly review and merge this pull request- https://github.com/ollelauribostrom/rebus/pull/691

Thanks!