Page MenuHomePhorge

Improving UX for ignoring timezone conflicts
ClosedPublic

Authored by roberto.urbani on Aug 24 2023, 15:37.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 11, 13:33
Unknown Object (File)
Thu, Apr 11, 10:40
Unknown Object (File)
Thu, Apr 11, 09:39
Unknown Object (File)
Thu, Apr 11, 01:56
Unknown Object (File)
Thu, Apr 11, 01:37
Unknown Object (File)
Thu, Apr 11, 01:27
Unknown Object (File)
Mon, Apr 1, 02:46
Unknown Object (File)
Mon, Apr 1, 02:46

Details

Summary

When there is a new timezone conflict, you will be able to ignore it with a checkbox.

Fix T15349

BeforeAfter
Adjust time - Before.png (239×822 px, 27 KB)
Adjust time - After.png (275×822 px, 31 KB)
Test Plan

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

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Adding another notification will only make this more annoying.

Why not just improve the current dialog to make it easier to ignore the conflict?

Adding another notification will only make this more annoying.

Why not just improve the current dialog to make it easier to ignore the conflict?

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

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).

Maybe we can improve the current popup from current:

Timezone Selection (including Ignore Conflict)
CancelChange Timezone

To (underlined the changed stuff):

Timezone Selection (without Ignore Conflict)
CancelIgnore ConflictChange Timezone

So, just a new button in the current popup

valerio.bozzolan retitled this revision from Improving UX for ignoring timezone conflicts Fix ref T15349 to Improving UX for ignoring timezone conflicts.Aug 24 2023, 18:37
valerio.bozzolan edited the summary of this revision. (Show Details)
valerio.bozzolan edited the test plan for this revision. (Show Details)

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
This revision now requires changes to proceed.Aug 28 2023, 08:06

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.

Updating D25420: Updating D25420: fixed tests error

roberto.urbani edited the test plan for this revision. (Show Details)

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
105

Can probably just use phutil_tag('strong', array(), 'OR') here.

avivey requested changes to this revision.Aug 29 2023, 08:44
avivey added inline comments.
src/applications/settings/controller/PhabricatorSettingsTimezoneController.php
111

Default to unchecked, and align the indentation.

This revision now requires changes to proceed.Aug 29 2023, 08:44
src/applications/settings/controller/PhabricatorSettingsTimezoneController.php
105

Or maybe make "Or" be the label for the checkbox (would show up under "New Setting").

Updating D25420:

  • Fixed indentation.
  • Default unchecked checkbox
  • Using label + checkbox instead of the "Or" message alone
This revision is now accepted and ready to land.Aug 29 2023, 12:50
src/applications/settings/controller/PhabricatorSettingsTimezoneController.php
16

@avivey Should we drop this line?

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.

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)

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)

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.

(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
93

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);

What about this small change? So, the checkbox has not a related "left" label, and has a more descriptive "right" label.


Ignore new setting.png (269×818 px, 28 KB)


If you want to try this, I can help amending this change. No problem.

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.

Hoping to be useful, I will land this after 2023-11-07 :) Thanks for this nice feature

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?

In D25420#12963, @speck wrote:

Instead of adding a checkbox is there precedent for having a separate button alongside Cancel and Submit?

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

Ah, thanks for landing

Any follow-up change is absolutely welcome to eventually implement any suggestion from here:

https://we.phorge.it/D25420#12963