When there is a new timezone conflict, you will be able to ignore it with a checkbox.
Fix T15349
Preview:
Details
Details
- Reviewers
valerio.bozzolan avivey - Group Reviewers
O1: Blessed Committers - Maniphest Tasks
- T15349: Explicitly cover in "Timezone settings" dialog how to ignore the browser vs profile conflict
Having a conflicting timezone, click the notification so the usual popup appears. There is a checkbox, leave it checked to ignore the current conflict, uncheck to manually resolve the conflict by selecting one of the available timezones.
Diff Detail
Diff Detail
- Repository
- rP Phorge
- Branch
- arcpatch-D25420_2
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 818 Build 818: arc lint + arc unit
Event Timeline
Comment Actions
Adding another notification will only make this more annoying.
Why not just improve the current dialog to make it easier to ignore the conflict?
Comment Actions
It would be one more click if we do it in the current dialog. The new notification is just one click and everything work; I'm for sure not a UX expert but this flow seems better to me
Comment Actions
But it adds another notification, and now the user will have to pick between 2 buttons that say "solve conflict".
It's also not the common path - in the common case, we expect users to actually approve the timezone offered by the dialog (which should be the one detected from the browser).
Comment Actions
Maybe we can improve the current popup from current:
| Timezone Selection (including Ignore Conflict) | |
| Cancel | Change Timezone |
To (underlined the changed stuff):
| Timezone Selection (without Ignore Conflict) | ||
| Cancel | Ignore Conflict | Change Timezone |
So, just a new button in the current popup
Comment Actions
I kindly mark this as "request changes" since the author said that it needs an update to reflect the screenshot. Feel free to update with
arc diff --update D25420
Comment Actions
Updating D25420: Added a checkbox to ignore timezone conflict in Adjust Timezone dialog window. Using the checkbox instead of a button because this way it's possible to reuse the existing form. Also, i still didn't find a way to capture the additional button click event. This way, if you check the checkbox, it will use the current setting timezone.
Comment Actions
if you check the checkbox, it will use the current setting timezone.
I don't understand what this means?
Also, can you post a screenshot of the dialog?
| src/applications/settings/controller/PhabricatorSettingsTimezoneController.php | ||
|---|---|---|
| 102 | Can probably just use phutil_tag('strong', array(), 'OR') here. | |
| src/applications/settings/controller/PhabricatorSettingsTimezoneController.php | ||
|---|---|---|
| 108 | Default to unchecked, and align the indentation. | |
| src/applications/settings/controller/PhabricatorSettingsTimezoneController.php | ||
|---|---|---|
| 102 | Or maybe make "Or" be the label for the checkbox (would show up under "New Setting"). | |
Comment Actions
Updating D25420:
- Fixed indentation.
- Default unchecked checkbox
- Using label + checkbox instead of the "Or" message alone
| src/applications/settings/controller/PhabricatorSettingsTimezoneController.php | ||
|---|---|---|
| 16 | 🤷♀️ | |
| src/applications/settings/controller/PhabricatorSettingsTimezoneController.php | ||
|---|---|---|
| 16 | xD @roberto.urbani If you want, this is a nice moment for you to decide whenever to keep this option, or just set as an empty array. Don't worry, in both cases someone out there in the planet will be furious. Feel free to roll a dice to decide. | |
Comment Actions
I like this approach, and prefer it to my simpler attempt from D25426; but have some minor suggestions:
- Instead of "You can ignore this conflict or adjust your profile setting to match your client", we should use the same order in the sentence as the options are shown to the user: "You can adjust your profile setting to match your client, or ignore this conflict."
- The label for the checkbox should be just "Ignore Conflict", without the "Or" at the start.
- When the "Ignore Conflict" checkbox is selected, the timezone dropdown should be disabled (and re-enabled if the checkbox is unmarked)
Comment Actions
I like these! Premising that the last part (hiding stuff) at the moment may be "somehow super-difficult" since at the moment this dialog is completely rendered by the server, and not by JavaScript.
So about this last part, maybe we can ignore at the moment, and create a sub-task for the future. But let's see other opinions.
Comment Actions
(I accepted but before landing please first review all not-done inline comments: https://we.phorge.it/differential/revision/inlines/25420/ and this comment: https://we.phorge.it/D25420#12264 )
| src/applications/settings/controller/PhabricatorSettingsTimezoneController.php | ||
|---|---|---|
| 88 | Our style-guide says that if the call doesn't fit in a single line, then all arguments should be on individual lines: $current_zone_formatted = phutil_tag( 'strong', 'array()', $current_zone_identifier); | |