Would it be easy to change the prompt to be more descriptive with actions? It’s generally more clear to have the buttons read “Discard changes” vs. “Ok” or “Yes”

In D25015#532, @speck wrote:

It’s generally more clear to have the buttons read “Discard changes” vs. “Ok” or “Yes”

I agree

Would it be easy to change the prompt to be more descriptive with actions?

For me, I think it might be on the hard side... and maybe add some bloat? Unless there is a styled generic confirm box created that maybe uses a callback instead of blocks execution...

Then instead of using native confirm(), it could be something new like

JX.confirm({
  message: "Form data may have changed. Are you sure you want to close this dialog?",
  buttonConfirm: "Discard Changes",
  buttonCancel: "Cancel",
  onConfirm: function() {
    // Something something
  }
});

I tried to find some example of simple dialogs. Surely there is a simple Confirm/Cancel one somewhere.. But the simplest example I can find so far is the "Meme" button in the compose area.

It looks like this comes from here: https://we.phorge.it/source/phorge/browse/master/src/applications/macro/controller/PhabricatorMacroMemeDialogController.php$66

$dialog = id(new AphrontDialogView())
   ->setUser($viewer)
   ->setTitle(pht('Create Meme'))
   ->appendForm($view)
   ->addCancelButton('/')
   ->addSubmitButton(pht('Llama Diorama'));

And this seems to be where it's generated: https://we.phorge.it/source/phorge/browse/master/src/view/AphrontDialogView.php

But an issue perhaps is that this is all generated with PHP... Unless we create a template AphrontDialogView() that just stays on the page or gets loaded in later, then have a Javelin method to populate it...

Unless that's how it already works with the dialog references here: https://we.phorge.it/source/phorge/browse/master/webroot/rsrc/externals/javelin/lib/Workflow.js

Basically I don't know if there's already a "right way" to do a confirm dialog via only JS or combination of JS and PHP which seems to be how dialogs are created...

As far the underlying issue itself... It seems that when it does happen to users, they tend to remember the time it happened very vividly.. There are multiple reports from the original thread, and I've heard reports as well..

On a related note, tonight I discovered this UIExamples application (changing links from my Vagrant install):

https://we.phorge.it/applications/view/PhabricatorUIExamplesApplication/

Which once installed would be available here: https://we.phorge.it/uiexample/

This could be worth expanding upon and referencing in developer documentation

In D25015#539, @Ekubischta wrote:

I 100% agree with this. Is there no way to invoke internationalization via JS only? If not, that's perfectly great, but wondering about an answer offhand.

One thought is to have a generic dialog sitting at all times with the proper internationalization ready.

Another thought... Can we represent this action with Unicode symbols instead of words?

There's no way to invoke pht from JS (because it's really complicated in the general case). The general practice is to provide the translated value from PHP when building the relevant objects. There should be an example in the UIExamples app - there's a page with buttons built in JS.

Using symbols (we have FontAwesome) is hard on non-visual users, so it should be limited.

The translated text can be a property of the dialogue itself, if we're worried about having extra objects laying around - the confirmation feels like it's a feature of the dialogue anyway.

+1 this has bitten me before.

In this case I think the error text is agnostic of the instance of the dialog. From looking at AphrontDialogView I didn't see any obvious way to include additional fields/text that could be pulled out here on the front end. Looking elsewhere in this file (line ~297) it looks like some other generic text is used

if (!this._paused) {
  JX.$E('Resuming a workflow which is not paused!');
}

However that's the only case where JX.$E is used outside of an is-development check. I'm not sure where these Javascript translations would go or when the translation file is generated.

In D25015#616, @speck wrote:

In this case I think the error text is agnostic of the instance of the dialog. From looking at AphrontDialogView I didn't see any obvious way to include additional fields/text that could be pulled out here on the front end. Looking elsewhere in this file (line ~297) it looks like some other generic text is used

if (!this._paused) {
  JX.$E('Resuming a workflow which is not paused!');
}

However that's the only case where JX.$E is used outside of an is-development check. I'm not sure where these Javascript translations would go or when the translation file is generated.

Got my hopes up thinking that it was a i18n-related function... but perhaps inspiring as one... I see that it's for exceptions. Does perhaps make a case for introducing the functionality and adding a solid structure for JS i18n as a future feature?

Oh, another potential concern...

What are the implications of making changes to the javelinjs.com https://github.com/phacility/javelin/ library?

Just do it in this context?

Ah I did not look closely enough. Dang. We should be attaching the message to the view on the server then, somehow.

I think trying to build l10n stuff on the client-side would be a large project. My guess is we would need to template the JavaScript, rendering the localized version on the server before serving to the client. Alternatively use different url paths for message translations but one thing that would b me tricky is minimizing what steps are for translators. I got the impression that upstream has ~4 languages but expected community/individuals to translate for their own needs. Introducing a different process for creating new translations on the client might get confusing for installations.

Also I don’t believe javelin.io is related to the Javalin used in Phab. The one used in Phab was written by, you may have guessed it, Evan (and possibly others at Facebook contributed). There’s another repo hosted on secure-dot for the Javalin JavaScript library.

Also I don’t believe javelin.io is related to the Javalin used in Phab.

Indeed... I had to update it... May be useful for a few minutes delay before emailing notifications :P Haha, just kidding there for now

Introducing a different process for creating new translations on the client might get confusing for installations.

Ideally this would happen in just one place, definitely something to bear in mind

What are the implications of making changes to the javelinjs.com https://github.com/phacility/javelin/ library?

This was folded into rP long ago - the separate repo was last updated in 2014. I wouldn't worry about it.

In this case I think the error text is agnostic of the instance of the dialog.

I was thinking about this use-case, which implies to me that the dialog author should be involved in the error text:

In D25015#633, @avivey wrote:

In this case I think the error text is agnostic of the instance of the dialog.

I was thinking about this use-case, which implies to me that the dialog author should be involved in the error text:

Ah this is a good point. We might need to build out arbitrary messages being attached to dialog views. I did a bit of searching but nothing stuck out at me for anything pre-existing.

It looks like there is a JX.phtize() which appears to be used to create a function that mimics pht() in JavaScript but I believe requires that whatever is passed to phtize() is effectively a map of translations which is presumably passed from the server somewhere. I've not yet uncovered this later part.

In D25015#678, @speck wrote:

It looks like there is a JX.phtize() which appears to be used to create a function that mimics pht() in JavaScript but I believe requires that whatever is passed to phtize() is effectively a map of translations which is presumably passed from the server somewhere. I've not yet uncovered this later part.

Nice find!!

It seems that an example of this being used is here: https://we.phorge.it/source/phorge/browse/master/src/applications/differential/view/DifferentialChangesetListView.php$252

Which calls the parent method here: https://we.phorge.it/source/phorge/browse/master/src/view/AphrontView.php$187

Ultimately calling this: https://we.phorge.it/source/phorge/browse/master/src/infrastructure/javelin/Javelin.php$5

Though it still hasn't clicked yet how it ties together and hands it off to the JS...

Found another reference to translations in JS as well: https://we.phorge.it/source/phorge/browse/master/webroot/rsrc/js/application/diff/DiffChangesetList.js$136

var pht = this.getTranslations();

But the odd thing here is that getTranslations() doesn't actually seem to be a JS function, at least that I can find so far... There is a method getTranslations() in the PHP though: https://we.phorge.it/source/phorge/browse/master/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php$10

@Leon95 updated/included some translated text for calendar widgets under D25016 which may also be useful as reference. However one big difference is that the calendar widget has its own explicit view/behavior, whereas front-end workflows have multiple varying uses with different content.

Noting the inline comment in the _send function regarding default error handling behavior I think there should be some way to pass in translation text to a workflow though I'm not exactly sure how/where. Maybe the initialize() function at the end of the file should be passed a pht object that can be used? I couldn't find where that initialize() function gets called in javascript though.

Dug up some more info as well...

Here is where the "+ Create Task" button is inserted in the menu on the Workboard:

https://we.phorge.it/source/phorge/browse/master/src/applications/project/controller/PhabricatorProjectBoardViewController.php$695

$specs = id(new ManiphestEditEngine())
  ->setViewer($viewer)
  ->newCreateActionSpecifications(array());

foreach ($specs as $spec) {
  $column_items[] = id(new PhabricatorActionView())
    ->setIcon($spec['icon'])
    ->setName($spec['name'])
    ->setHref($spec['uri'])
    ->setDisabled($spec['disabled'])
    ->addSigil('column-add-task')
    ->setMetadata(
      array(
        'createURI' => $spec['uri'],
        'columnPHID' => $column->getPHID(),
        'boardPHID' => $project->getPHID(),
        'projectPHID' => $default_phid,
      ));
}

$column_items[] = id(new PhabricatorActionView())
  ->setType(PhabricatorActionView::TYPE_DIVIDER);

$query_uri = urisprintf('viewquery/%d/', $column->getID());
$query_uri = $state->newWorkboardURI($query_uri);

$column_items[] = id(new PhabricatorActionView())
  ->setName(pht('View Tasks as Query'))
  ->setIcon('fa-search')
  ->setHref($query_uri);

I included the separator and next item ("View Tasks as Query") to show that it's a bit different in how it's inserted, apparently there can be groups within the first separated section:

/**
 * Build a raw description of available "Create New Object" UI options so
 * other methods can build menus or buttons.
 */
public function newCreateActionSpecifications(array $parameters) {
  ...
    $specs[] = array(
      'name' => $this->getObjectCreateShortText(),
      'uri' => $create_uri,
      'icon' => $menu_icon,
      'disabled' => $disabled,
      'workflow' => $workflow,
    );
  ...
  return $specs;
}

Here is a dump of the $specs array:

/srv/phorge/phorge/src/applications/project/controller/PhabricatorProjectBoardViewController.php:695:
array (size=1)
  0 => 
    array (size=5)
      'name' => string 'Create Task' (length=11)
      'uri' => string '/maniphest/task/edit/form/default/' (length=34)
      'icon' => string 'fa-plus' (length=7)
      'disabled' => boolean false
      'workflow' => boolean false

The setMetadata() method may be of interest here... Perhaps there can be a new array key there for cancelDialogPrompt, something like that...

So that's an idea for a place to get the translation from PHP to JS in the Workboard context, through the button... The form itself seems to be the standard Maniphest Create Task form via https://we.phorge.it/source/phorge/browse/master/src/applications/maniphest/editor/ManiphestEditEngine.php

The text "Create Task" appears *only* in that file in getObjectCreateShortText() https://we.phorge.it/source/phorge/browse/master/src/applications/maniphest/editor/ManiphestEditEngine.php$52

Yet the dialog title is the same, "Create Task"...

Somehow it knows to load this form, and put that title in the dialog header... perhaps it would be possible to tie this together by adding metadata to the button

What if, to get this functionality pushed through, we for now change the verbiage to two English words -- "Discard changes?"

And then come back to l18n later?

I may be late for the party, but can't the translated verbiage be provided to the dialog in the $form_attributes in AphrontDialogView.php:337, and read using form.getAttribute(key) in the js?

In D25015#735, @avivey wrote:

I may be late for the party, but can't the translated verbiage be provided to the dialog in the $form_attributes in AphrontDialogView.php:337, and read using form.getAttribute(key) in the js?

I looked at the $form_attributes a week or so back but I think those end up being transformed into the literal HTML attributes of the dialog's <form> element. I think we need to add a new field to the dialog, something like setMetadata() that @dcog found in the workboard view.

Thanks -- Can anyone think of a more generic solution here as far as dialogs? Something for both confirm() and prompt().

One thing to note here: Native JavaScript blocks execution when these functions are used. This is apparently nearly impossible behavior to mimic exactly, but we can solve this with asynchronous functions with our generic and platform-specific confirm() and alert() functions. Otherwise, should there be an always-available global translation for both alert() and confirm()?

In D25015#792, @dcog wrote:

Thanks -- Can anyone think of a more generic solution here as far as dialogs? Something for both confirm() and prompt().

One thing to note here: Native JavaScript blocks execution when these functions are used. This is apparently nearly impossible behavior to mimic exactly, but we can solve this with asynchronous functions with our generic and platform-specific confirm() and alert() functions. Otherwise, should there be an always-available global translation for both alert() and confirm()?

Example of custom confirm():

JX.confirm({
  message: "Form data may have changed. Are you sure you want to close this dialog?",
  buttonConfirm: "Discard Changes",
  buttonCancel: "Cancel",
  onConfirm: function() {
    // Something something
  }
});

But it would need translation added...

In D25015#793, @dcog wrote:

In D25015#792, @dcog wrote:

Thanks -- Can anyone think of a more generic solution here as far as dialogs? Something for both confirm() and prompt().

One thing to note here: Native JavaScript blocks execution when these functions are used. This is apparently nearly impossible behavior to mimic exactly, but we can solve this with asynchronous functions with our generic and platform-specific confirm() and alert() functions. Otherwise, should there be an always-available global translation for both alert() and confirm()?

Example of custom confirm():

JX.confirm({
  message: "Form data may have changed. Are you sure you want to close this dialog?",
  buttonConfirm: "Discard Changes",
  buttonCancel: "Cancel",
  onConfirm: function() {
    // Something something
  }
});

But it would need translation added...

This uses an onConfirm() method in our generic mechanism for producing such a dialog.... It just would need translation

We can have a i18n option for both confirmDialog and alertDialog.... Let's do this, who is down?

I'm down to do a https://meet.jit.si/ with someone else who is also down for this idea as well as the idea of translatable generic confirm() and alert() dialogs that can trivially go from pht() in PHP to JavaScript. Anyone?

I feel like we should move translation to a new revision, so we can move forward with this one.

Yes - I know I started us down this rabbit hole with my earlier comment - Sorry!

If we can land this, it solves a giant issue (the accidental closing of dialogs) and then let's task translation on the Javascript side in another task / revision

Thoughts?

I think that makes sense. Could you make a task to address this so we don’t lose track of it? Then let’s get this landed.

In D25015#826, @speck wrote:

I think that makes sense. Could you make a task to address this so we don’t lose track of it? Then let’s get this landed.

Yep - done T15053