fix custom key with multiple characters #32
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
furcan
requested changes
Dec 4, 2021
There was a problem hiding this comment.
Hi,
.gitignorefile changes should be discarded.package-lock.jsonfile should be removed.yarn.lockfile changes should be discarded.- I've added a suggestion for the fixes that you have committed.
src/kioskboard.js
In addition,
No worries about the distribution, the contribution guideline is not ready yet. So, only the fixes are enough in this case.
Thanks,
Furkan.
package-lock.json
Outdated
| @@ -0,0 +1,7849 @@ | |||
| { | |||
There was a problem hiding this comment.
Please remove this file as well. We are using yarn instead of npm.
yarn.lock
Outdated
| @@ -2,29 +2,29 @@ | |||
| # yarn lockfile v1 | |||
src/kioskboard.js
Outdated
Comment on lines
650
to
654
| e.preventDefault(); | ||
|
|
||
| // update the selectionStart | ||
| theInputSelIndex = input.selectionStart || (input.value || '').length; | ||
| // Math.min prevents out of bound, theInputSelIndex cannot be greater than the length of theInputValArray | ||
| theInputSelIndex = Math.min((input.selectionStart || (input.value || '').length),theInputValArray.length); |
There was a problem hiding this comment.
These changes should be discarded if my suggestion on the adding part is ok for you.
src/kioskboard.js
Outdated
Comment on lines
601
to
607
| theInputValArray.splice(theInputSelIndex, 0, keyValue); | ||
| /** Prevents breaking on custom keys that insert multiple characters, for example, imagine a ".com" key, | ||
| * it would be inserted into the array as a single element and could no longer be deleted because the length of | ||
| * the input would be greater than the length of the array, adding Math.min (and comparing the respective lengths) | ||
| * to the backspace key prevents this problem but would delete the entire contents of the key (all characters, for example ".com") | ||
| * Instead, concatenate the main array of the contents with a possible array of keyValue, you can delete any single character. | ||
| **/ | ||
| theInputValArray = theInputValArray.concat(keyValue.split('')); |
There was a problem hiding this comment.
We need to use and update the selection index for adding each character because of the allowRealKeyboard option.
So, my suggestion is:
- Spliting the
keyValuefor the possibility of the multiple characters. - Creating a
forloop for the new array of thekeyValue. - add, update, and dispatch events will be moved in this loop.
keyElm.addEventListener('click', function (e) {
e.preventDefault();
// check input max & maxlength
var maxLength = (input.getAttribute('maxlength') || '') * 1;
var max = (input.getAttribute('max') || '') * 1;
var liveValueLength = (input.value || '').length || 0;
if (maxLength > 0 && liveValueLength >= maxLength) { return false; }
if (max > 0 && liveValueLength >= max) { return false; }
// input trigger focus
input.focus();
// key's value
var keyValue = e.currentTarget.dataset.value || '';
// check capslock
if (isCapsLockActive) {
keyValue = keyValue.toLocaleUpperCase(keyboardLanguage);
} else {
keyValue = keyValue.toLocaleLowerCase(keyboardLanguage);
}
var keyValArr = keyValue.split('');
for (var keyValIndex = 0; keyValIndex < keyValArr.length; keyValIndex++) {
// update the selectionStart
theInputSelIndex = input.selectionStart || (input.value || '').length;
// add value by index
theInputValArray.splice(theInputSelIndex, 0, keyValArr[keyValIndex]);
// update input value
input.value = theInputValArray.join('');
// set next selection index
input.setSelectionRange(theInputSelIndex + 1, theInputSelIndex + 1);
// input trigger change event for update the value
input.dispatchEvent(changeEvent);
}
}, false);What do you think?
There was a problem hiding this comment.
Do you want to replace the keyElm.addEventListener('click', function (e) {... code block(above) with your current one and update this MR? (with discarding other ones)
|
Hi @skullab , |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix for issue #31
NOTE:
no distribution version created