Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SiteSettings: Add timezone section to general settings. #4226

Merged
merged 10 commits into from
Mar 29, 2016

Conversation

retrofox
Copy link
Contributor

Related issue: #3902

This PR adds a timezones dropdown to general settings which allows to the user customize the timezone of his site.

image

Testing

  1. Select your testing site and go to general settings page. The url should be something like http://calypso.localhost:3000/settings/general/

  2. Select a timezone from the dropdown and save the site

  3. The previous timezone should be there after a hard-refresh

  4. Go to "Create a new post", open the post schedule component and pay attention to the timezone section. The current site timezone should be there with the difference in hours between you.

image

@retrofox retrofox added this to the Stark Iteration 0 milestone Mar 22, 2016
@retrofox retrofox force-pushed the update/site-settings-timezone branch 4 times, most recently from 0b01813 to 6fe8b39 Compare March 24, 2016 13:13
@retrofox retrofox changed the title WIP - SiteSettings: Add timezone section to general settings. SiteSettings: Add timezone section to general settings. Mar 24, 2016
@retrofox retrofox force-pushed the update/site-settings-timezone branch from 6fe8b39 to 266ec01 Compare March 24, 2016 19:34
@retrofox retrofox added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Mar 24, 2016
if ( this.state.isOpen ) {
this.closeDropdown();
}

if ( this.props.initialSelected !== nextProps.initialSelected ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the initialSelected prop changes after the user has made a selection? Should we retain the user's selection rather than overriding it? I'd be annoyed if I had to make my selection again. It looks like state.selected will be undefined until a selection is made, so could we add a check for typeof this.state.selected === 'undefined' before we setState?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 smart

@retrofox retrofox force-pushed the update/site-settings-timezone branch from 8a30208 to f43efb7 Compare March 28, 2016 21:44
/>

<FormSettingExplanation>
{ this.translate( 'Choose a city in the same timezone as you.' ) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simpler: Choose a city in your timezone.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 just FYI it was copied from wp-admin

@retrofox retrofox force-pushed the update/site-settings-timezone branch from f43efb7 to 6690b4e Compare March 29, 2016 11:54
@retrofox retrofox force-pushed the update/site-settings-timezone branch from 6690b4e to 8a1354e Compare March 29, 2016 12:00
@rralian
Copy link
Contributor

rralian commented Mar 29, 2016

Looks and works great for me!

@rralian rralian added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 29, 2016
@retrofox
Copy link
Contributor Author

thanks @rralian

@retrofox retrofox merged commit 28e0ea8 into master Mar 29, 2016
@retrofox retrofox deleted the update/site-settings-timezone branch March 29, 2016 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants