From 0d34efd2cadedb93141177555454af2aaebc599a Mon Sep 17 00:00:00 2001 From: Priyesh Shah Date: Thu, 16 Feb 2023 04:32:34 +1100 Subject: [PATCH 1/6] disabled timezone options if it is set to automatic --- .../settings/Profile/TimezoneSelectPage.js | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/pages/settings/Profile/TimezoneSelectPage.js b/src/pages/settings/Profile/TimezoneSelectPage.js index fd91f9a2e91f..2b4e23d87622 100644 --- a/src/pages/settings/Profile/TimezoneSelectPage.js +++ b/src/pages/settings/Profile/TimezoneSelectPage.js @@ -34,7 +34,7 @@ class TimezoneSelectPage extends Component { this.saveSelectedTimezone = this.saveSelectedTimezone.bind(this); this.filterShownTimezones = this.filterShownTimezones.bind(this); - this.currentSelectedTimezone = lodashGet(props.currentUserPersonalDetails, 'timezone.selected', CONST.DEFAULT_TIME_ZONE.selected); + this.timezone = lodashGet(props.currentUserPersonalDetails, 'timezone', CONST.DEFAULT_TIME_ZONE); this.allTimezones = _.chain(moment.tz.names()) .filter(timezone => !timezone.startsWith('Etc/GMT')) .map(timezone => ({ @@ -42,19 +42,40 @@ class TimezoneSelectPage extends Component { keyForList: timezone, // Include the green checkmark icon to indicate the currently selected value - customIcon: timezone === this.currentSelectedTimezone ? greenCheckmark : undefined, + customIcon: timezone === this.timezone.selected ? greenCheckmark : undefined, // This property will make the currently selected value have bold text - boldStyle: timezone === this.currentSelectedTimezone, + boldStyle: timezone === this.timezone.selected, })) .value(); this.state = { - timezoneInputText: this.currentSelectedTimezone, + timezoneInputText: this.timezone.selected, timezoneOptions: this.allTimezones, }; } + componentDidUpdate() { + // Update timezoneInputText & all timezoneOptions when the timezone object changes + const newTimezone = lodashGet(this.props.currentUserPersonalDetails, 'timezone', CONST.DEFAULT_TIME_ZONE); + if (_.isEqual(this.timezone, newTimezone)) { return; } + this.timezone = newTimezone; + const updatedAllTimezones = _.map(this.allTimezones, timezone => ({ + ...timezone, + + // Include the green checkmark icon to indicate the currently selected value + customIcon: timezone === this.timezone.selected ? greenCheckmark : undefined, + + // This property will make the currently selected value have bold text + boldStyle: timezone === this.timezone.selected, + })); + + this.setState({ + timezoneInputText: this.timezone.selected, + timezoneOptions: updatedAllTimezones, + }); + } + /** * @param {Object} timezone * @param {String} timezone.text @@ -90,7 +111,7 @@ class TimezoneSelectPage extends Component { onChangeText={this.filterShownTimezones} onSelectRow={this.saveSelectedTimezone} optionHoveredStyle={styles.hoveredComponentBG} - sections={[{data: this.state.timezoneOptions}]} + sections={[{data: this.state.timezoneOptions, isDisabled: this.timezone.automatic}]} shouldHaveOptionSeparator safeAreaPaddingBottomStyle={safeAreaPaddingBottomStyle} /> From 073f8f75422731c219f64fb10db053adfa42d7e7 Mon Sep 17 00:00:00 2001 From: Priyesh Shah Date: Fri, 17 Feb 2023 02:40:00 +1100 Subject: [PATCH 2/6] updated the comment in componentDidUpdate --- src/pages/settings/Profile/TimezoneSelectPage.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/settings/Profile/TimezoneSelectPage.js b/src/pages/settings/Profile/TimezoneSelectPage.js index 2b4e23d87622..97b5a23fece3 100644 --- a/src/pages/settings/Profile/TimezoneSelectPage.js +++ b/src/pages/settings/Profile/TimezoneSelectPage.js @@ -56,7 +56,8 @@ class TimezoneSelectPage extends Component { } componentDidUpdate() { - // Update timezoneInputText & all timezoneOptions when the timezone object changes + // componentDidUpdate is added in order to update the timezone options when automatic is toggled on/off as + // navigating back doesn't unmount the page, thus it won't update the timezone options & stay disabled without this. const newTimezone = lodashGet(this.props.currentUserPersonalDetails, 'timezone', CONST.DEFAULT_TIME_ZONE); if (_.isEqual(this.timezone, newTimezone)) { return; } this.timezone = newTimezone; From 9e22ee5adc2e28c5c10dca2b433de097bf5216e8 Mon Sep 17 00:00:00 2001 From: Priyesh Shah Date: Fri, 17 Feb 2023 06:51:02 +1100 Subject: [PATCH 3/6] refactored componentDidUpdate to fix options not udating issue --- .../settings/Profile/TimezoneSelectPage.js | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/pages/settings/Profile/TimezoneSelectPage.js b/src/pages/settings/Profile/TimezoneSelectPage.js index 97b5a23fece3..a2ec14d38131 100644 --- a/src/pages/settings/Profile/TimezoneSelectPage.js +++ b/src/pages/settings/Profile/TimezoneSelectPage.js @@ -39,7 +39,7 @@ class TimezoneSelectPage extends Component { .filter(timezone => !timezone.startsWith('Etc/GMT')) .map(timezone => ({ text: timezone, - keyForList: timezone, + keyForList: this.getKey(timezone), // Include the green checkmark icon to indicate the currently selected value customIcon: timezone === this.timezone.selected ? greenCheckmark : undefined, @@ -59,24 +59,38 @@ class TimezoneSelectPage extends Component { // componentDidUpdate is added in order to update the timezone options when automatic is toggled on/off as // navigating back doesn't unmount the page, thus it won't update the timezone options & stay disabled without this. const newTimezone = lodashGet(this.props.currentUserPersonalDetails, 'timezone', CONST.DEFAULT_TIME_ZONE); - if (_.isEqual(this.timezone, newTimezone)) { return; } + if (_.isEqual(this.timezone, newTimezone)) { + return; + } this.timezone = newTimezone; - const updatedAllTimezones = _.map(this.allTimezones, timezone => ({ - ...timezone, + this.allTimezones = _.map(this.allTimezones, (timezone) => { + const text = timezone.text.split('-')[0]; + return { + text, + keyForList: this.getKey(text), - // Include the green checkmark icon to indicate the currently selected value - customIcon: timezone === this.timezone.selected ? greenCheckmark : undefined, + // Include the green checkmark icon to indicate the currently selected value + customIcon: text === this.timezone.selected ? greenCheckmark : undefined, - // This property will make the currently selected value have bold text - boldStyle: timezone === this.timezone.selected, - })); + // This property will make the currently selected value have bold text + boldStyle: text === this.timezone.selected, + }; + }); this.setState({ timezoneInputText: this.timezone.selected, - timezoneOptions: updatedAllTimezones, + timezoneOptions: this.allTimezones, }); } + /** + * @param {String} text + * @return {string} key for list item + */ + getKey(text) { + return `${text}-${(new Date()).getTime()}`; + } + /** * @param {Object} timezone * @param {String} timezone.text From 3f587b6dac1805466395136aa1726eec556cf9c7 Mon Sep 17 00:00:00 2001 From: Priyesh Shah Date: Fri, 17 Feb 2023 06:55:11 +1100 Subject: [PATCH 4/6] refactored get timezone code to follow DRY --- src/pages/settings/Profile/TimezoneSelectPage.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/pages/settings/Profile/TimezoneSelectPage.js b/src/pages/settings/Profile/TimezoneSelectPage.js index a2ec14d38131..fd8af1ab4836 100644 --- a/src/pages/settings/Profile/TimezoneSelectPage.js +++ b/src/pages/settings/Profile/TimezoneSelectPage.js @@ -34,7 +34,7 @@ class TimezoneSelectPage extends Component { this.saveSelectedTimezone = this.saveSelectedTimezone.bind(this); this.filterShownTimezones = this.filterShownTimezones.bind(this); - this.timezone = lodashGet(props.currentUserPersonalDetails, 'timezone', CONST.DEFAULT_TIME_ZONE); + this.timezone = this.getUserTimezone(props.currentUserPersonalDetails); this.allTimezones = _.chain(moment.tz.names()) .filter(timezone => !timezone.startsWith('Etc/GMT')) .map(timezone => ({ @@ -58,7 +58,7 @@ class TimezoneSelectPage extends Component { componentDidUpdate() { // componentDidUpdate is added in order to update the timezone options when automatic is toggled on/off as // navigating back doesn't unmount the page, thus it won't update the timezone options & stay disabled without this. - const newTimezone = lodashGet(this.props.currentUserPersonalDetails, 'timezone', CONST.DEFAULT_TIME_ZONE); + const newTimezone = this.getUserTimezone(this.props.currentUserPersonalDetails); if (_.isEqual(this.timezone, newTimezone)) { return; } @@ -91,6 +91,14 @@ class TimezoneSelectPage extends Component { return `${text}-${(new Date()).getTime()}`; } + /** + * @param {Object} currentUserPersonalDetails + * @return {Object} user's timezone data + */ + getUserTimezone(currentUserPersonalDetails) { + return lodashGet(currentUserPersonalDetails, 'timezone', CONST.DEFAULT_TIME_ZONE); + } + /** * @param {Object} timezone * @param {String} timezone.text From 953ca2ea2f3e809c190f62b3457f7aab4f5f72bb Mon Sep 17 00:00:00 2001 From: Priyesh Shah Date: Mon, 20 Feb 2023 02:10:29 +1100 Subject: [PATCH 5/6] Code refactoring to follow DRY & add comments --- .../settings/Profile/TimezoneSelectPage.js | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/pages/settings/Profile/TimezoneSelectPage.js b/src/pages/settings/Profile/TimezoneSelectPage.js index fd8af1ab4836..f64714026f82 100644 --- a/src/pages/settings/Profile/TimezoneSelectPage.js +++ b/src/pages/settings/Profile/TimezoneSelectPage.js @@ -33,20 +33,12 @@ class TimezoneSelectPage extends Component { this.saveSelectedTimezone = this.saveSelectedTimezone.bind(this); this.filterShownTimezones = this.filterShownTimezones.bind(this); + this.getTimezoneOption = this.getTimezoneOption.bind(this); this.timezone = this.getUserTimezone(props.currentUserPersonalDetails); this.allTimezones = _.chain(moment.tz.names()) .filter(timezone => !timezone.startsWith('Etc/GMT')) - .map(timezone => ({ - text: timezone, - keyForList: this.getKey(timezone), - - // Include the green checkmark icon to indicate the currently selected value - customIcon: timezone === this.timezone.selected ? greenCheckmark : undefined, - - // This property will make the currently selected value have bold text - boldStyle: timezone === this.timezone.selected, - })) + .map(timezone => this.getTimezoneOption(timezone)) .value(); this.state = { @@ -65,16 +57,7 @@ class TimezoneSelectPage extends Component { this.timezone = newTimezone; this.allTimezones = _.map(this.allTimezones, (timezone) => { const text = timezone.text.split('-')[0]; - return { - text, - keyForList: this.getKey(text), - - // Include the green checkmark icon to indicate the currently selected value - customIcon: text === this.timezone.selected ? greenCheckmark : undefined, - - // This property will make the currently selected value have bold text - boldStyle: text === this.timezone.selected, - }; + return this.getTimezoneOption(text); }); this.setState({ @@ -84,6 +67,7 @@ class TimezoneSelectPage extends Component { } /** + * We add the current time to the key to fix a bug where the list options don't update unless the key is updated. * @param {String} text * @return {string} key for list item */ @@ -91,6 +75,24 @@ class TimezoneSelectPage extends Component { return `${text}-${(new Date()).getTime()}`; } + /** + * Get timezone option object for the list. + * @param {String} text + * @return {Object} Timezone list option + */ + getTimezoneOption(text) { + return { + text, + keyForList: this.getKey(text), + + // Include the green checkmark icon to indicate the currently selected value + customIcon: text === this.timezone.selected ? greenCheckmark : undefined, + + // This property will make the currently selected value have bold text + boldStyle: text === this.timezone.selected, + }; + } + /** * @param {Object} currentUserPersonalDetails * @return {Object} user's timezone data From 97c20b32cec80b809f0d179eb36e8d2e64c35ff9 Mon Sep 17 00:00:00 2001 From: Priyesh Shah Date: Mon, 20 Feb 2023 02:19:47 +1100 Subject: [PATCH 6/6] minor code cleanup --- src/pages/settings/Profile/TimezoneSelectPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/settings/Profile/TimezoneSelectPage.js b/src/pages/settings/Profile/TimezoneSelectPage.js index f64714026f82..b050a965ebfb 100644 --- a/src/pages/settings/Profile/TimezoneSelectPage.js +++ b/src/pages/settings/Profile/TimezoneSelectPage.js @@ -38,7 +38,7 @@ class TimezoneSelectPage extends Component { this.timezone = this.getUserTimezone(props.currentUserPersonalDetails); this.allTimezones = _.chain(moment.tz.names()) .filter(timezone => !timezone.startsWith('Etc/GMT')) - .map(timezone => this.getTimezoneOption(timezone)) + .map(this.getTimezoneOption) .value(); this.state = {