key callbacks: Allow stopPropagation when other

Catch alls should not trap all key events
salesforce · Nov 30, 2018 · f90c9eb · f90c9eb
1 parent d12ce75
commit f90c9eb
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 13 deletions.
diff --git a/components/combobox/combobox.jsx b/components/combobox/combobox.jsx
@@ -529,8 +529,15 @@ class Combobox extends React.Component {
 		};
 
 		if (this.props.variant === 'readonly') {
-			callbacks[KEYS.TAB] = { callback: this.handleKeyDownTab };
-			callbacks.other = { callback: this.handleKeyDownOther };
+			if (this.props.selection.length > 2) {
+				callbacks[KEYS.TAB] = { callback: this.handleKeyDownTab };
+			} else {
+				callbacks[KEYS.TAB] = undefined;
+			}
+			callbacks.other = {
+				callback: this.handleKeyDownOther,
+				stopPropagation: false,
+			};
 		}
 
 		// Helper function that takes an object literal of callbacks that are triggered with a key event

diff --git a/components/pill-container/private/selected-listbox.jsx b/components/pill-container/private/selected-listbox.jsx
@@ -180,14 +180,7 @@ const SelectedListBox = (props) =>
 				aria-label={props.assistiveText.selectedListboxLabel}
 			>
 				{props.selection.map((option, renderIndex) => {
-					// Makes first pill in DOM snapshots have aria-selected=true on first render
-					const setActiveBasedOnstateFromParent =
-						renderIndex === props.activeOptionIndex &&
-						isEqual(option, props.activeOption);
-					const listboxRenderedForFirstTime =
-						props.activeOptionIndex === -1 && renderIndex === 0;
-					const active =
-						setActiveBasedOnstateFromParent || listboxRenderedForFirstTime;
+					const hasTabIndex = renderIndex === props.activeOptionIndex;
 					const icon = getIcon(option);
 					const avatar = !icon ? getAvatar(option) : null;
 
@@ -198,7 +191,7 @@ const SelectedListBox = (props) =>
 							key={`${props.id}-list-item-${option.id}`}
 						>
 							<Pill
-								active={active}
+								active={hasTabIndex && props.listboxHasFocus}
 								assistiveText={{
 									remove: props.assistiveText.removePill,
 								}}
@@ -234,7 +227,7 @@ const SelectedListBox = (props) =>
 									removeTitle: props.labels.removePillTitle,
 								}}
 								requestFocus={props.listboxHasFocus}
-								tabIndex={active ? 0 : -1}
+								tabIndex={hasTabIndex ? 0 : -1}
 							/>
 						</li>
 					);

diff --git a/utilities/key-callbacks.js b/utilities/key-callbacks.js
@@ -26,7 +26,8 @@ const mapKeyEventCallbacks = (
 		}
 		callbacks[event.keyCode].callback(event, callbacks[event.keyCode].data);
 	} else if (event.keyCode && callbacks.other) {
-		if (stopPropagation) {
+		// You will likely NOT want to stop propagation of all key presses!
+		if (callbacks.other.stopPropagation) {
 			EventUtil.trapEvent(event);
 		}
 		callbacks.other.callback(event, callbacks.other.data);