Merge pull request JedWatson#2179 from yuri-sakharov/fix/search-for-i…

…nvalid-label-value Fixed search for invalid label and/or value
sushant-at-nitor · Nov 28, 2017 · 75f3043 · 75f3043
2 parents 0d0ccd3 + 7e10e30
commit 75f3043
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 11 deletions.
diff --git a/src/utils/defaultFilterOptions.js b/src/utils/defaultFilterOptions.js
@@ -1,6 +1,10 @@
 import stripDiacritics from './stripDiacritics';
 import trim from './trim';
 
+function isValid(value) {
+	return typeof (value) !== 'undefined' && value !== null && value !== '';
+}
+
 function filterOptions (options, filterValue, excludeOptions, props) {
 	if (props.ignoreAccents) {
 		filterValue = stripDiacritics(filterValue);
@@ -20,24 +24,35 @@ function filterOptions (options, filterValue, excludeOptions, props) {
 		if (excludeOptions && excludeOptions.indexOf(option[props.valueKey]) > -1) return false;
 		if (props.filterOption) return props.filterOption.call(this, option, filterValue);
 		if (!filterValue) return true;
-		var valueTest = String(option[props.valueKey]);
-		var labelTest = String(option[props.labelKey]);
+
+		var value = option[props.valueKey];
+		var label = option[props.labelKey];
+		var hasValue = isValid(value);
+		var hasLabel = isValid(label);
+
+		if (!hasValue && !hasLabel) {
+			return false;
+		}
+
+		var valueTest = hasValue ? String(value) : null;
+		var labelTest = hasLabel ? String(label) : null;
 
 		if (props.ignoreAccents) {
-			if (props.matchProp !== 'label') valueTest = stripDiacritics(valueTest);
-			if (props.matchProp !== 'value') labelTest = stripDiacritics(labelTest);
+			if (valueTest && props.matchProp !== 'label') valueTest = stripDiacritics(valueTest);
+			if (labelTest && props.matchProp !== 'value') labelTest = stripDiacritics(labelTest);
 		}
 
 		if (props.ignoreCase) {
-			if (props.matchProp !== 'label') valueTest = valueTest.toLowerCase();
-			if (props.matchProp !== 'value') labelTest = labelTest.toLowerCase();
+			if (valueTest && props.matchProp !== 'label') valueTest = valueTest.toLowerCase();
+			if (labelTest && props.matchProp !== 'value') labelTest = labelTest.toLowerCase();
 		}
+
 		return props.matchPos === 'start' ? (
-			(props.matchProp !== 'label' && valueTest.substr(0, filterValue.length) === filterValue) ||
-			(props.matchProp !== 'value' && labelTest.substr(0, filterValue.length) === filterValue)
+			(valueTest && props.matchProp !== 'label' && valueTest.substr(0, filterValue.length) === filterValue) ||
+			(labelTest && props.matchProp !== 'value' && labelTest.substr(0, filterValue.length) === filterValue)
 		) : (
-			(props.matchProp !== 'label' && valueTest.indexOf(filterValue) >= 0) ||
-			(props.matchProp !== 'value' && labelTest.indexOf(filterValue) >= 0)
+			(valueTest && props.matchProp !== 'label' && valueTest.indexOf(filterValue) >= 0) ||
+			(labelTest && props.matchProp !== 'value' && labelTest.indexOf(filterValue) >= 0)
 		);
 	});
 }

diff --git a/test/Select-test.js b/test/Select-test.js
@@ -848,7 +848,11 @@ describe('Select', () => {
 				{ value: 20, label: 'Twenty' },
 				{ value: 21, label: 'Twenty-one' },
 				{ value: 34, label: 'Thirty-four' },
-				{ value: 54, label: 'Fifty-four' }
+				{ value: 54, label: 'Fifty-four' },
+				{ value: null, label: null },
+				{ fish: 'salomon', type: 'inedible' },
+				{ value: 0, type: 'inedible' },
+				{ label: 0, type: 'inedible' },
 			];
 
 			describe('with matchPos=any and matchProp=any', () => {
@@ -880,6 +884,22 @@ describe('Select', () => {
 							expect.it('to have text', 'Fifty-four')
 						]);
 				});
+
+				it('should not match text when value and/or label are invalid for search', () => {
+					typeSearchText('ined');
+					expect(ReactDOM.findDOMNode(instance), 'to contain elements matching',
+						'.Select-noresults');
+					expect(ReactDOM.findDOMNode(instance), 'to contain no elements matching',
+						'.Select-option');
+				});
+
+				it('should not match text when value and/or label are null', () => {
+					typeSearchText('null');
+					expect(ReactDOM.findDOMNode(instance), 'to contain elements matching',
+						'.Select-noresults');
+					expect(ReactDOM.findDOMNode(instance), 'to contain no elements matching',
+						'.Select-option');
+				});
 			});
 
 			describe('with matchPos=start and matchProp=any', () => {