Skip to content

Commit

Permalink
Merge pull request #14826 from Expensify/jack-revertOPtionsListChanges
Browse files Browse the repository at this point in the history
Revert 'fix: Timezone changes to some 1st option in the list while pressing the enter key'
  • Loading branch information
deetergp authored Feb 3, 2023
2 parents db77957 + 04915ad commit abd4d80
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 47 deletions.
9 changes: 3 additions & 6 deletions src/components/OptionsList/BaseOptionsList.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class BaseOptionsList extends Component {
* @returns {Array<Object>}
*/
buildFlatSectionArray() {
const optionHeight = variables.optionRowHeight;
let offset = 0;

// Start with just an empty list header
Expand All @@ -120,12 +121,8 @@ class BaseOptionsList extends Component {

// Add section items
for (let i = 0; i < section.data.length; i++) {
let fullOptionHeight = variables.optionRowHeight;
if (i > 0 && this.props.shouldHaveOptionSeparator) {
fullOptionHeight += variables.borderTopWidth;
}
flatArray.push({length: fullOptionHeight, offset});
offset += fullOptionHeight;
flatArray.push({length: optionHeight, offset});
offset += optionHeight;
}

// Add the section footer
Expand Down
37 changes: 4 additions & 33 deletions src/components/OptionsSelector/BaseOptionsSelector.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,10 @@ class BaseOptionsSelector extends Component {
this.relatedTarget = null;

const allOptions = this.flattenSections();
const focusedIndex = this.getInitiallyFocusedIndex(allOptions);

this.state = {
allOptions,
focusedIndex,
focusedIndex: this.props.shouldTextInputAppearBelowOptions ? allOptions.length : 0,
};
}

Expand Down Expand Up @@ -86,8 +85,6 @@ class BaseOptionsSelector extends Component {
true,
);

this.scrollToIndex(this.state.focusedIndex, false);

if (!this.props.autoFocus) {
return;
}
Expand Down Expand Up @@ -139,25 +136,6 @@ class BaseOptionsSelector extends Component {
}
}

/**
* @param {Array<Object>} allOptions
* @returns {Number}
*/
getInitiallyFocusedIndex(allOptions) {
const defaultIndex = this.props.shouldTextInputAppearBelowOptions ? allOptions.length : 0;
if (_.isUndefined(this.props.initiallyFocusedOptionKey)) {
return defaultIndex;
}

const indexOfInitiallyFocusedOption = _.findIndex(allOptions, option => option.keyForList === this.props.initiallyFocusedOptionKey);

if (indexOfInitiallyFocusedOption >= 0) {
return indexOfInitiallyFocusedOption;
}

return defaultIndex;
}

/**
* Flattens the sections into a single array of options.
* Each object in this array is enhanced to have:
Expand Down Expand Up @@ -198,9 +176,8 @@ class BaseOptionsSelector extends Component {
* Scrolls to the focused index within the SectionList
*
* @param {Number} index
* @param {Boolean} animated
*/
scrollToIndex(index, animated = true) {
scrollToIndex(index) {
const option = this.state.allOptions[index];
if (!this.list || !option) {
return;
Expand All @@ -219,7 +196,7 @@ class BaseOptionsSelector extends Component {
}
}

this.list.scrollToLocation({sectionIndex: adjustedSectionIndex, itemIndex, animated});
this.list.scrollToLocation({sectionIndex: adjustedSectionIndex, itemIndex});
}

/**
Expand Down Expand Up @@ -293,13 +270,7 @@ class BaseOptionsSelector extends Component {
showTitleTooltip={this.props.showTitleTooltip}
isDisabled={this.props.isDisabled}
shouldHaveOptionSeparator={this.props.shouldHaveOptionSeparator}
onLayout={() => {
this.scrollToIndex(this.state.focusedIndex, false);

if (this.props.onLayout) {
this.props.onLayout();
}
}}
onLayout={this.props.onLayout}
/>
) : <FullScreenLoadingIndicator />;
return (
Expand Down
4 changes: 0 additions & 4 deletions src/components/OptionsSelector/optionsSelectorPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ const propTypes = {

/** Whether to show a line separating options in list */
shouldHaveOptionSeparator: PropTypes.bool,

/** Key of the option that we should focus on when first opening the options list */
initiallyFocusedOptionKey: PropTypes.string,
};

const defaultProps = {
Expand All @@ -116,7 +113,6 @@ const defaultProps = {
disableArrowKeysActions: false,
isDisabled: false,
shouldHaveOptionSeparator: false,
initiallyFocusedOptionKey: undefined,
};

export {propTypes, defaultProps};
3 changes: 1 addition & 2 deletions src/pages/settings/Profile/TimezoneSelectPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,8 @@ class TimezoneSelectPage extends Component {
onChangeText={this.filterShownTimezones}
onSelectRow={this.saveSelectedTimezone}
optionHoveredStyle={styles.hoveredComponentBG}
sections={[{data: this.state.timezoneOptions, indexOffset: 0}]}
sections={[{data: this.state.timezoneOptions}]}
shouldHaveOptionSeparator
initiallyFocusedOptionKey={this.currentSelectedTimezone}
/>
</ScreenWrapper>
);
Expand Down
2 changes: 1 addition & 1 deletion src/styles/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -1770,7 +1770,7 @@ const styles = {
},

borderTop: {
borderTopWidth: variables.borderTopWidth,
borderTopWidth: 1,
borderColor: themeColors.border,
},

Expand Down
1 change: 0 additions & 1 deletion src/styles/variables.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,4 @@ export default {
checkboxLabelActiveOpacity: 0.7,
avatarChatSpacing: 12,
chatInputSpacing: 52, // 40 + avatarChatSpacing
borderTopWidth: 1,
};

0 comments on commit abd4d80

Please sign in to comment.