@awight Sorry for the last minute changes. Updated the wording and added a mock for the second warning. Let me know if anything is unclear!

Very nice, just in time! Good to know that we definitely want to update the back button workflow to make it consistent.

I'm not sure we considered the "is edited" test when estimating this task. This turns out to be new functionality, the back button test was only checking for *any* template parameter values.

Change 752654 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] Replace confirmation overlay with a popup dialog

https://gerrit.wikimedia.org/r/752654

I'm not sure we considered the "is edited" test when estimating this task. This turns out to be new functionality, the back button test was only checking for *any* template parameter values.

I don't think the functionality needs to change. Maybe it's the wording of the warning that needs to be more specific? It was meant to reflect that parameters were added and values inputted, which I think were the criteria we used when adding the warning previously? What do you think it's implying that isn't happening?

In T297792#7608807, @ECohen_WMDE wrote:

I'm not sure we considered the "is edited" test when estimating this task. This turns out to be new functionality, the back button test was only checking for *any* template parameter values.

I don't think the functionality needs to change. Maybe it's the wording of the warning that needs to be more specific? It was meant to reflect that parameters were added and values inputted, which I think were the criteria we used when adding the warning previously? What do you think it's implying that isn't happening?

The test we use for the back button is to go through each parameter, check whether it has a value, and compare this value with the autovalue and the default. If it's any other value, this means the user must have editing the parameter value and therefore there are unsaved changes to lose. Basically, the implementation takes a shortcut which is now adding tech debt. Agreed that both the back and close actions should use logic like "did the user change any parameter values", and 1 I'd like to make the implementation as consistent as possible across the two actions, favoring the newer logic as you describe it here.

My comment is mostly just a heads-up that implementation might be slightly more complex than we had imagined, the logic you described is clear and I plan to follow the outline.

Ok - I'm just not sure if the logic needs to change? Because what you described sounds like what I expected. With the back button that's the only edit possible since the situation only applies to new templates and it's not really possible to "change" anything since it loads empty (other than auto values and defaults).

Maybe I'm misunderstanding something, since we seem to be on a similar page - I just don't want to accidentally over-complicate this ticket.

(Discussed in chat, the ticket is correct and we'll continue with improvements to change-detection logic.)

Change 752666 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] Confirm before closing template dialog

https://gerrit.wikimedia.org/r/752666

While reviewing this I stumbled a bit over All changes will be lost and cannot be undone.. Personally I would find it more clear if the text somewhat emphasizes on pending changes. Just to make clear, that the "original" values still remain.

While writing this, I also wonder if All could be confusing as well. A user could open the template editor to edit the same transclusion several times in one edit session. The user could apply some of the changes and then reopen the editor to do other changes, that he then cancels. So maybe just Pending changes...?

I guess it's a bit different when inserting and going back to the search. But maybe it also won't hurt to change it to Go back to template search? All pending edits.... Just to be as specific as possible.

No hard feelings though. ;-)

Hmm good points - I wonder if it's actually simplest to just remove the "all" so it would be ... Changes will be lost and cannot be undone. and ... Edits will be lost and cannot be undone. I'm not sure if the word "pending" is needed because I think edit or change implies that it's the new stuff only that will be affected. Pending actually implies to me that maybe there could be some other changes in addition to my recent changes, which might cause more confusion?

Maybe removing "all" is enough to make it a bit clearer. I would start there - will update the task description!

In T297792#7613095, @ECohen_WMDE wrote:

Hmm good points - I wonder if it's actually simplest to just remove the "all" so it would be ... Changes will be lost and cannot be undone. and ... Edits will be lost and cannot be undone. I'm not sure if the word "pending" is needed because I think edit or change implies that it's the new stuff only that will be affected. Pending actually implies to me that maybe there could be some other changes in addition to my recent changes, which might cause more confusion?

Maybe removing "all" is enough to make it a bit clearer. I would start there - will update the task description!

Sounds good! Thanks for looking into it.

Change 752654 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Replace confirmation overlay with a popup dialog

https://gerrit.wikimedia.org/r/752654

Would it be possible to change the label wording? Instead of Cancel, it would be better if it could say Keep editing. Was reading an article on best practices, and it said that "Cancel" should be avoided because it can cause additional confusion. Especially when a user is intending to do a destructive action, they might think that's the progressive one when trying to just move through the pop-up quickly.

Sorry about another last minute wording change!

In T297792#7615617, @ECohen_WMDE wrote:

Instead of Cancel, it would be better if it could say Keep editing. Was reading an article on best practices, and it said that "Cancel" should be avoided because it can cause additional confusion. Especially when a user is intending to do a destructive action, they might think that's the progressive one when trying to just move through the pop-up quickly.

Brilliant! Making the change now...

Change 752666 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Confirm before closing template dialog

https://gerrit.wikimedia.org/r/752666

Responding to the example shared on mattermost here to keep everything related in one place: F34916515

Interesting example, though this is definitely the one that should be changed though in terms of both focus and wording (removing the "Are you sure?" stuff, a problem I've also seen elsewhere on-wiki). If ours hasn't been shared for translation yet we could switch to "Continue editing" instead of "Keep editing" for consistency, but otherwise I would keep ours following best practices since I've seen other examples of confirmation dialogs also following it. Hopefully Codex helps resolve this and creates consistency and guidelines for confirmation dialogs.

In T297792#7618875, @ECohen_WMDE wrote:

If ours hasn't been shared for translation yet we could switch to "Continue editing" instead of "Keep editing" for consistency

It's barely gone out, and I think this might not matter for translation since it has the same meaning. I'll change the text.

Change 753694 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] Change confirmation-reject text for consistency

https://gerrit.wikimedia.org/r/753694

Change 753694 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Change confirmation-reject text for consistency

https://gerrit.wikimedia.org/r/753694

Edge case: editing an existing template and deleting the last parameter's value causes the close box to not emit a warning.

Another edge case: using the <esc> key causes the user to see the close-box confirmation even if they're inserting a new template and should be in the back-button workflow.