search

backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 39 forks source link

Set default country based on selected timezone #344

Closed ghost closed 9 years ago

ghost commented 9 years ago

The Default Country setting was removed from the installer (see https://github.com/backdrop/backdrop-issues/issues/12) but users can still set a default timezone there. We should set a default value for the Default Country setting (still available on the Regional Settings page) based on the selected timezone.

ghost commented 9 years ago

See the pull request for details on how this is addressed.

A more complete solution that solves this for timezones where the country name isn't listed would be to lookup a timezone's city/area and find the matching country, but as there is no core ability to do this type of lookup I don't think it's worth implementing for this small helper feature.

quicksketch commented 9 years ago

A more complete solution that solves this for timezones where the country name isn't listed would be to lookup a timezone's city/area and find the matching country, but as there is no core ability to do this type of lookup I don't think it's worth implementing for this small helper feature.

Ha, I wrote the same thing in the PR itself.

I'm not sure about the effectiveness of this approach, as I noted in the PR:

Timezones typically have the name of the continent rather than the name of the country. e.g. "America/New_York" or "Europe/Amsterdam". Pretty much the only place this would work is Australia, where the continent and country are the same thing.

Like you say, we don't have a mapping between cities and countries, making this task challenging without it.

ghost commented 9 years ago

There are a few other places this does work (e.g. 'Pacific/Guam') but yeah, a better approach is needed. Will look into it.

I have been meaning to ask this (and this seems like a good time): should discussions about a PR's code go in the PR itself or the parent issue?

quicksketch commented 9 years ago

I have been meaning to ask this (and this seems like a good time): should discussions about a PR's code go in the PR itself or the parent issue?

Approach and discussion should attempt to be focused in the issue. And the PR should be discussion of the code submitted. That's the goal... but in reality it's hard to keep track of. Just keep in mind that PRs can be closed and replaced several times before an issue is solved, so the main thread of conversation should stick to the main issue, gluing it all together if multiple PRs are submitted.

ghost commented 9 years ago

Latest commit adds a function to standard.inc that matches timezones with country codes. This was mainly based off an export from GeoNames with missing data obtained from Time Zone Converter and Wikipedia.

Also, similar to core country list, I added an alter function to allow adding/changing timezone countries.

quicksketch commented 9 years ago

Merged in https://github.com/backdrop/backdrop/pull/482. Thanks so much!

Long-term, @jenlampton and I discussed if we need this setting at all. It's not used by core (or contrib) as far as I could find, but the intention was to provide the setting for future use, though that never happened AFAIK. However, I'm all for making it more convenient immediately. We can decide if the whole setting provides value at a later time. For now, I think this is nice to keep around and now will hardly ever need to be set manually. Great job, excellent work on the PR.