When there is a new timezone conflict, you will be able to ignore it with a checkbox.
Fix T15349
| Before | After |
|---|---|
 |  |
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
- Commits
- rP37ecdf233697: Improving UX for ignoring timezone conflicts
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); | |
Comment Actions
What about this small change? So, the checkbox has not a related "left" label, and has a more descriptive "right" label.
If you want to try this, I can help amending this change. No problem.
Comment Actions
Hoping to be useful, implement tips, to be easily tested.
https://we.phorge.it/D25420#12747
If you don't like this, say so, and I can revert for you.
If you like this final version, feel free to land.
Comment Actions
Hoping to be useful, I will land this after 2023-11-07 :) Thanks for this nice feature
Comment Actions
Instead of adding a checkbox is there precedent for having a separate button alongside Cancel and Submit?
The checkbox is essentially a “Do not ask/warn me again” option which could potentially apply to many forms, however the behavior is slightly different. I think this current state could allow the form to get into a confusing state (select a value and click checkbox).
I think it would be more straightforward if instead of a checkbox it’s a new/different submit button. Or maybe the cancel button should just behave that way?
Comment Actions
Yeah interesting question. I remember I inspected AphrontDialogView without much success about that. I suppose it supports just Cancel or Confirm.
https://we.phorge.it/source/phorge/browse/master/src/view/AphrontDialogView.php
The checkbox is essentially a “Do not ask/warn me again” option which could potentially apply to many forms, however the behavior is slightly different. I think this current state could allow the form to get into a confusing state (select a value and click checkbox).
I think it would be more straightforward if instead of a checkbox it’s a new/different submit button. Or maybe the cancel button should just behave that way?
I've seen that the Cancel button usually is about just closing the Popup or going to another page without persisting anything. So at the moment we have:
- Cancel → don't persist anything, the popup is still shown forever
- Confirm → persist something. If you have the checkbox, it persist your preference to ignore this. You don't see the popup anymore.
If I understand correctly, you suggest to persist in both cases, so:
- Ignore → persist to ignore the thing
- Confirm → persist to select the thing
I have not a strong opinion about this. At the moment this may be a better compromise. Less "standard" (since the Cancel does not just close the popup) but indeed more intuitive to me
Comment Actions
Ah, thanks for landing
Any follow-up change is absolutely welcome to eventually implement any suggestion from here: