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

Choices don't allow Integer type for "value" #527

Closed
KungFuLucky7 opened this issue Feb 21, 2019 · 4 comments
Closed

Choices don't allow Integer type for "value" #527

KungFuLucky7 opened this issue Feb 21, 2019 · 4 comments

Comments

@KungFuLucky7
Copy link

KungFuLucky7 commented Feb 21, 2019

I had to update the stripHTML in src/scripts/lib/utils.js to allow Integer type values:

-export const stripHTML = html =>
-  html
-    .replace(/&/g, '&')
-    .replace(/>/g, '&rt;')
-    .replace(/</g, '&lt;')
-    .replace(/"/g, '&quot;');
+export const stripHTML = html => {
+  if (typeof html === 'string') {
+    return html
+      .replace(/&/g, '&amp;')
+      .replace(/>/g, '&rt;')
+      .replace(/</g, '&lt;')
+      .replace(/"/g, '&quot;');
+  } else {
+    return html;
+  }
+};
@KungFuLucky7 KungFuLucky7 changed the title Choices don't allow for Integer type for "value" Choices don't allow Integer type for "value" Feb 21, 2019
@ghazal
Copy link

ghazal commented Feb 22, 2019

I do fully agree with this.
Otherwise it throws an error.
My solution was to add:
return String(html).replace(/&/g, '&amp;').replace(/>/g, '&rt;').replace(/</g, '&lt;').replace(/"/g, '&quot;');
on line 270 - choices.js
But I guess this solution is much better.

@jshjohnson
Copy link
Collaborator

I understand this is an issue but the fix shouldn't be within stripHTML - stripHTML should only be called if the value is a string. Will review any PRs

@KungFuLucky7
Copy link
Author

KungFuLucky7 commented Feb 22, 2019

It seems not as DRY'd for a fix if done outside of stripHTML:

diff --git a/src/scripts/components/input.js b/src/scripts/components/input.js
index a26c0d9..76d1276 100644
--- a/src/scripts/components/input.js
+++ b/src/scripts/components/input.js
@@ -25,7 +25,10 @@ export default class Input {
   }
 
   get value() {
-    return stripHTML(this.element.value);
+    if (typeof this.element.value === 'string') {
+      return stripHTML(this.element.value);
+    }
+    return this.element.value;
   }

   addEventListeners() {
diff --git a/src/scripts/constants.js b/src/scripts/constants.js
index 14f42ef..388fea7 100644
--- a/src/scripts/constants.js
+++ b/src/scripts/constants.js
@@ -65,7 +65,10 @@ export const DEFAULT_CONFIG = {
   itemSelectText: 'Press to select',
   uniqueItemText: 'Only unique values can be added',
   customAddItemText: 'Only values matching specific conditions can be added',
-  addItemText: value => `Press Enter/Tab to add <b>"${stripHTML(value)}"</b>`,
+  addItemText: value =>
+    `Press Enter/Tab to add <b>"${
+      typeof value === 'string' ? stripHTML(value) : value
+    }"</b>`,
   maxItemText: maxItemCount => `Only ${maxItemCount} values can be added`,
   itemComparer: (choice, item) => choice === item,
   fuseOptions: {
diff --git a/src/scripts/lib/utils.js b/src/scripts/lib/utils.js
index 4c1bec2..73e24e7 100644
--- a/src/scripts/lib/utils.js
+++ b/src/scripts/lib/utils.js
@@ -169,7 +169,7 @@ export const calcWidthOfInput = (input, callback) => {
   let width = input.offsetWidth;
 
   if (value) {
-    const testEl = strToEl(`<span>${stripHTML(value)}</span>`);
+    const testEl = strToEl(`<span>${typeof value === 'string' ? stripHTML(value) : value}</span>`);
     testEl.style.position = 'absolute';
     testEl.style.padding = '0';
     testEl.style.top = '-9999px';

@ghazal
Copy link

ghazal commented Feb 23, 2019

@jshjohnson
it seems that after the corrections you made in v. 6.0.2, the script is not generating errors anymore (ie this particular bug).
Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants