Skip to content

Commit

Permalink
Merge pull request #29084 from s-alves10/fix/issue-22812
Browse files Browse the repository at this point in the history
fix: prevent refocusing in options-selector and selection-list
  • Loading branch information
MonilBhavsar authored Oct 10, 2023
2 parents 50de088 + 1b94af4 commit 81565ec
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 15 deletions.
5 changes: 5 additions & 0 deletions src/components/OptionRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ const propTypes = {
/** Whether to remove the lateral padding and align the content with the margins */
shouldDisableRowInnerPadding: PropTypes.bool,

/** Whether to prevent default focusing on select */
shouldPreventDefaultFocusOnSelectRow: PropTypes.bool,

/** Whether to wrap large text up to 2 lines */
isMultilineSupported: PropTypes.bool,

Expand All @@ -95,6 +98,7 @@ const defaultProps = {
style: null,
shouldHaveOptionSeparator: false,
shouldDisableRowInnerPadding: false,
shouldPreventDefaultFocusOnSelectRow: false,
};

class OptionRow extends Component {
Expand Down Expand Up @@ -213,6 +217,7 @@ class OptionRow extends Component {
hoverDimmingValue={1}
hoverStyle={this.props.hoverStyle}
needsOffscreenAlphaCompositing={lodashGet(this.props.option, 'icons.length', 0) >= 2}
onMouseDown={this.props.shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined}
>
<View style={sidebarInnerRowStyle}>
<View style={[styles.flexRow, styles.alignItemsCenter]}>
Expand Down
2 changes: 2 additions & 0 deletions src/components/OptionsList/BaseOptionsList.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ function BaseOptionsList({
showScrollIndicator,
listContainerStyles,
shouldDisableRowInnerPadding,
shouldPreventDefaultFocusOnSelectRow,
disableFocusOptions,
canSelectMultipleOptions,
shouldShowMultipleOptionSelectorAsButton,
Expand Down Expand Up @@ -206,6 +207,7 @@ function BaseOptionsList({
isDisabled={isItemDisabled}
shouldHaveOptionSeparator={index > 0 && shouldHaveOptionSeparator}
shouldDisableRowInnerPadding={shouldDisableRowInnerPadding}
shouldPreventDefaultFocusOnSelectRow={shouldPreventDefaultFocusOnSelectRow}
isMultilineSupported={isRowMultilineSupported}
/>
);
Expand Down
4 changes: 4 additions & 0 deletions src/components/OptionsList/optionsListPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ const propTypes = {
/** Whether to disable the inner padding in rows */
shouldDisableRowInnerPadding: PropTypes.bool,

/** Whether to prevent default focusing when selecting a row */
shouldPreventDefaultFocusOnSelectRow: PropTypes.bool,

/** Whether to show the scroll bar */
showScrollIndicator: PropTypes.bool,

Expand Down Expand Up @@ -107,6 +110,7 @@ const defaultProps = {
onLayout: undefined,
shouldHaveOptionSeparator: false,
shouldDisableRowInnerPadding: false,
shouldPreventDefaultFocusOnSelectRow: false,
showScrollIndicator: false,
isRowMultilineSupported: false,
};
Expand Down
7 changes: 4 additions & 3 deletions src/components/OptionsSelector/BaseOptionsSelector.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ class BaseOptionsSelector extends Component {
*/
selectRow(option, ref) {
return new Promise((resolve) => {
if (this.props.shouldShowTextInput && this.props.shouldFocusOnSelectRow) {
if (this.props.shouldShowTextInput && this.props.shouldPreventDefaultFocusOnSelectRow) {
if (this.relatedTarget && ref === this.relatedTarget) {
this.textInput.focus();
this.relatedTarget = null;
Expand Down Expand Up @@ -344,7 +344,7 @@ class BaseOptionsSelector extends Component {
* @param {Object} option
*/
addToSelection(option) {
if (this.props.shouldShowTextInput && this.props.shouldFocusOnSelectRow) {
if (this.props.shouldShowTextInput && this.props.shouldPreventDefaultFocusOnSelectRow) {
this.textInput.focus();
if (this.textInput.isFocused()) {
setSelection(this.textInput, 0, this.props.value.length);
Expand Down Expand Up @@ -372,7 +372,7 @@ class BaseOptionsSelector extends Component {
maxLength={this.props.maxLength}
keyboardType={this.props.keyboardType}
onBlur={(e) => {
if (!this.props.shouldFocusOnSelectRow) {
if (!this.props.shouldPreventDefaultFocusOnSelectRow) {
return;
}
this.relatedTarget = e.relatedTarget;
Expand Down Expand Up @@ -417,6 +417,7 @@ class BaseOptionsSelector extends Component {
isLoading={!this.props.shouldShowOptions}
showScrollIndicator={this.props.showScrollIndicator}
isRowMultilineSupported={this.props.isRowMultilineSupported}
shouldPreventDefaultFocusOnSelectRow={this.props.shouldPreventDefaultFocusOnSelectRow}
/>
);
return (
Expand Down
6 changes: 3 additions & 3 deletions src/components/OptionsSelector/optionsSelectorPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ const propTypes = {
/** Whether to show the title tooltip */
showTitleTooltip: PropTypes.bool,

/** Whether to focus the textinput after an option is selected */
shouldFocusOnSelectRow: PropTypes.bool,
/** Whether to prevent default focusing of options and focus the textinput when selecting an option */
shouldPreventDefaultFocusOnSelectRow: PropTypes.bool,

/** Whether to autofocus the search input on mount */
autoFocus: PropTypes.bool,
Expand Down Expand Up @@ -144,7 +144,7 @@ const defaultProps = {
hideSectionHeaders: false,
boldStyle: false,
showTitleTooltip: false,
shouldFocusOnSelectRow: false,
shouldPreventDefaultFocusOnSelectRow: false,
autoFocus: true,
shouldShowConfirmButton: false,
confirmButtonText: undefined,
Expand Down
12 changes: 11 additions & 1 deletion src/components/SelectionList/BaseListItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,16 @@ import RadioListItem from './RadioListItem';
import OfflineWithFeedback from '../OfflineWithFeedback';
import CONST from '../../CONST';

function BaseListItem({item, isFocused = false, isDisabled = false, showTooltip, canSelectMultiple = false, onSelectRow, onDismissError = () => {}}) {
function BaseListItem({
item,
isFocused = false,
isDisabled = false,
showTooltip,
shouldPreventDefaultFocusOnSelectRow = false,
canSelectMultiple = false,
onSelectRow,
onDismissError = () => {},
}) {
const isUserItem = lodashGet(item, 'icons.length', 0) > 0;
const ListItem = isUserItem ? UserListItem : RadioListItem;

Expand All @@ -32,6 +41,7 @@ function BaseListItem({item, isFocused = false, isDisabled = false, showTooltip,
hoverDimmingValue={1}
hoverStyle={styles.hoveredComponentBG}
dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
onMouseDown={shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined}
>
<View
style={[
Expand Down
8 changes: 5 additions & 3 deletions src/components/SelectionList/BaseSelectionList.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function BaseSelectionList({
showScrollIndicator = false,
showLoadingPlaceholder = false,
showConfirmButton = false,
shouldFocusOnSelectRow = false,
shouldPreventDefaultFocusOnSelectRow = false,
isKeyboardShown = false,
inputRef = null,
disableKeyboardShortcuts = false,
Expand Down Expand Up @@ -211,14 +211,14 @@ function BaseSelectionList({

onSelectRow(item);

if (shouldShowTextInput && shouldFocusOnSelectRow && textInputRef.current) {
if (shouldShowTextInput && shouldPreventDefaultFocusOnSelectRow && textInputRef.current) {
textInputRef.current.focus();
}
};

const selectAllRow = () => {
onSelectAll();
if (shouldShowTextInput && shouldFocusOnSelectRow && textInputRef.current) {
if (shouldShowTextInput && shouldPreventDefaultFocusOnSelectRow && textInputRef.current) {
textInputRef.current.focus();
}
};
Expand Down Expand Up @@ -299,6 +299,7 @@ function BaseSelectionList({
canSelectMultiple={canSelectMultiple}
onSelectRow={() => selectRow(item, true)}
onDismissError={onDismissError}
shouldPreventDefaultFocusOnSelectRow={shouldPreventDefaultFocusOnSelectRow}
/>
);
};
Expand Down Expand Up @@ -401,6 +402,7 @@ function BaseSelectionList({
accessibilityState={{checked: flattenedSections.allSelected}}
disabled={flattenedSections.allOptions.length === flattenedSections.disabledOptionsIndexes.length}
dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
onMouseDown={shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined}
>
<Checkbox
accessibilityLabel={translate('workspace.people.selectAll')}
Expand Down
5 changes: 3 additions & 2 deletions src/components/SelectionList/selectionListPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ const radioListItemPropTypes = {
const baseListItemPropTypes = {
...commonListItemPropTypes,
item: PropTypes.oneOfType([PropTypes.shape(userListItemPropTypes.item), PropTypes.shape(radioListItemPropTypes.item)]),
shouldPreventDefaultFocusOnSelectRow: PropTypes.bool,
};

const propTypes = {
Expand Down Expand Up @@ -167,8 +168,8 @@ const propTypes = {
/** Whether to show the default confirm button */
showConfirmButton: PropTypes.bool,

/** Whether to focus the textinput after an option is selected */
shouldFocusOnSelectRow: PropTypes.bool,
/** Whether to prevent default focusing of options and focus the textinput when selecting an option */
shouldPreventDefaultFocusOnSelectRow: PropTypes.bool,

/** A ref to forward to the TextInput */
inputRef: PropTypes.oneOfType([PropTypes.object]),
Expand Down
2 changes: 1 addition & 1 deletion src/pages/NewChatPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ function NewChatPage({betas, isGroupChat, personalDetails, reports, translate})
onChangeText={setSearchTerm}
headerMessage={headerMessage}
boldStyle
shouldFocusOnSelectRow={!Browser.isMobile()}
shouldPreventDefaultFocusOnSelectRow={!Browser.isMobile()}
shouldShowOptions={isOptionsDataReady}
shouldShowConfirmButton
confirmButtonText={selectedOptions.length > 1 ? translate('newChatPage.createGroup') : translate('newChatPage.createChat')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ function MoneyRequestParticipantsSelector({
textInputLabel={translate('optionsSelector.nameEmailOrPhoneNumber')}
safeAreaPaddingBottomStyle={safeAreaPaddingBottomStyle}
shouldShowOptions={isOptionsDataReady}
shouldFocusOnSelectRow={!Browser.isMobile()}
shouldPreventDefaultFocusOnSelectRow={!Browser.isMobile()}
shouldDelayFocus
/>
</View>
Expand Down
2 changes: 2 additions & 0 deletions src/pages/workspace/WorkspaceInvitePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import withPolicy, {policyDefaultProps, policyPropTypes} from './withPolicy';
import FullPageNotFoundView from '../../components/BlockingViews/FullPageNotFoundView';
import ROUTES from '../../ROUTES';
import * as PolicyUtils from '../../libs/PolicyUtils';
import * as Browser from '../../libs/Browser';
import useNetwork from '../../hooks/useNetwork';
import useLocalize from '../../hooks/useLocalize';
import SelectionList from '../../components/SelectionList';
Expand Down Expand Up @@ -231,6 +232,7 @@ function WorkspaceInvitePage(props) {
onConfirm={inviteUser}
showScrollIndicator
showLoadingPlaceholder={!didScreenTransitionEnd || !OptionsListUtils.isPersonalDetailsReady(props.personalDetails)}
shouldPreventDefaultFocusOnSelectRow={!Browser.isMobile()}
/>
<View style={[styles.flexShrink0]}>
<FormAlertWithSubmitButton
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/WorkspaceMembersPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ function WorkspaceMembersPage(props) {
onDismissError={dismissError}
showLoadingPlaceholder={!isOfflineAndNoMemberDataAvailable && (!OptionsListUtils.isPersonalDetailsReady(props.personalDetails) || _.isEmpty(props.policyMembers))}
showScrollIndicator
shouldFocusOnSelectRow={!Browser.isMobile()}
shouldPreventDefaultFocusOnSelectRow={!Browser.isMobile()}
inputRef={textInputRef}
/>
</View>
Expand Down

0 comments on commit 81565ec

Please sign in to comment.