Page MenuHomePhabricator

Show message in core's edit conflict interface about the new UI
Closed, ResolvedPublic1 Estimated Story Points

Description

Show a message (yellow banner style) on top of the old interface about the possibility to have the new interface.

Wording:
"An updated interface for solving edit conflicts is now available. Enable the Two Column Edit Conflict View in your [[preferences (link to user's Editing preferences section)]] to try it out."

Acceptance criteria

  • Show the message to all users using the old interface
  • Allow the user to dismiss this message (E.g. via a cookie)

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

@Lena_WMDE I think it was decided in a past meeting that this banner is unnecessary because the audience doesn't exist (disabled settings were not transferred to the default). Should we mark it as resolved?

Lena_WMDE closed this task as Resolved.EditedApr 9 2020, 8:03 AM
Lena_WMDE claimed this task.

@ecohen For small default wikis (dewiki, fawiki, arwiki), that's correct. However, we still have to roll this out on all other wikis and, arguably, we should transfer the preferences for those wikis.

Lena_WMDE reopened this task as Open.Apr 9 2020, 8:59 AM
awight added a subscriber: awight.Apr 17 2020, 7:59 AM

@Lena_WMDE We can discriminate between users who opted-out of the beta feature, vs. users who opted out using the new preference. I'd like to only show popups to the first audience, if that makes sense to you and @ecohen ?

The proposed wording,

"An updated interface for solving edit conflicts is now available. Change your preferences here (link) to try it out."

might not give the user enough information to easily find which preference to enable for the new interface. They'll land on the Editing Preferences tab, which has 20 or so options. Should we add a clue "An updated interface for solving edit conflicts is now available. Change your preferences here(link) to try out the Two Column Conflict Edit View." ?

We also have the option of directly setting the preference when the link is clicked, but that's probably not a good UX.

We can discriminate between users who opted-out of the beta feature, vs. users who opted out using the new preference.

Just realizing that I was wrong about this. We can discriminate only until they open and save their preferences, at which point the beta opt-out will be carried over to become the new preference.

Considering that, I'd like to double-, triple-check that we want this feature. It's only going to be displayed to people who have explicitly opted-out, I don't see much value and do see it causing a lot of annoyance.

We can discriminate only until they open and save their preferences, at which point the beta opt-out will be carried over to become the new preference.

@awight
I'm not sure I understand this sentence. Who is it possible to show the message to and who not?

@awight the proposed wording in the ticket is already: "An updated interface for solving edit conflicts is now available. Enable the Two Column Edit Conflict View in your [[preferences (link to user's Editing preferences section)]] to try it out." That should give them enough information. I'm not sure where you got the wording you referenced but it is probably just an outdated version.

I also don't follow who would see this message and who would not, great if you could explain!

@awight the proposed wording in the ticket is already: "An updated interface for solving edit conflicts is now available. Enable the Two Column Edit Conflict View in your [[preferences (link to user's Editing preferences section)]] to try it out." That should give them enough information. I'm not sure where you got the wording you referenced but it is probably just an outdated version.

Whew--I found that outdated text in the comments because I'm not reading carefully--I see the new text now, thank you!

I also don't follow who would see this message and who would not, great if you could explain!

Here are the alternatives as I see it:

A) Banner only appears for users who have opted out of the beta preference, but have no new preference set. (And hides when dissmiss cookie is present.)

  • This is my favorite, but unfortunately is impossible for a technical reason, because the new preference gets updated from the old preference at some random time (the first time the user visits and saves preferences on the "Editing" tab, whether or not they have checked or unchecked the TwoCol box).

B) Banner appears to everyone in the legacy workflow. (And hides with dismiss cookie.)

  • I believe this is what the task is currently requesting. This feels wrong to me because people who opt-out using the new preference are going to see the banner on their next conflict and be sad. But I'm happy to defer to other people's judgment, just making sure we're clear on the behavior.

C) Banner appears on some random percentage of conflicts, until the cookie is set

  • We haven't discussed this yet, but it might be more polite? Especially in case a user blocks cookies, etc.

No matter which of the above we choose, I think we have to exclude no-JS users because the cookie mechanism won't work for them.

Thanks for the clear explanation @awight! As discussed, even though Option A would be preferable we'll go with Option B because Option A is not feasible.

Just thinking about this and had an idea. Is there any way to see how recently the preference was set? Then it is not tied to beta vs default settings, but just if they opted out recently. This could help avoid someone who just made the change being annoyed by the message.

If not, then I agree with Lena it should be a pretty unintrusive banner with the option to dismiss, so it's not such a problem to show it to anyone using the old UI.

We discussed this elsewhere, and since it's not possible to detect when a user updated their preferences we'll go ahead with Option B.

WMDE-Fisch moved this task from Sprint Backlog to Doing on the WMDE-QWERTY-Sprint-2020-04-15 board.

While looking into the implementation of this I realized, that the the explanatory text and visual above the "default" edit conflict screen varies a lot depending on the wiki. I just wanted give some examples. It could mean, that in some cases our "hint" to the newer interface is a bigger "eye catcher" than the the text that explains what's generally going on.

I'm not sure if that generally would mean that we should be even more discreet with our hint. IMO the default explanatory edit conflict intro text is way _too_ discreet, but I guess it's quite a process to change something about that.

Default layout and text:

English Wikipedia overwritten version of text and layout:

Spanish Wikipedia overwritten version of text and layout:

Thanks for catching this. Yes, it's unfortunate but I think out of our control.

The one thing that we can do is make the message a bit more neutral. Let's use the OOUI MessageWidget (type: 'notice'), which is gray. I would use it without a title though, with just the text in the ticket.

Maybe I miss something here, but how should the dismissal work? A little cross somewhere in the upper right corner? A button? Thanks @ecohen :-)

P.S.: Currently we will have something like this:

Also note that due to the contrast the link to the preferences - hidden in the word preferences - is not very visible. Maybe we write that link out so it says in your Special:Preferences (with Special:Preferences being the link)?

Also one question to @Lena_WMDE I'm not sure if this was discussed yet. Assuming that this note goes away eventually when the feature was default everywhere long enough, I guess its ok to risk some temporary technical debt and we could hard code the current small set of wikis that have the feature as default and exclude them from showing this note. ( because there everyone was forced to see the new feature and could opt out of it )

Any bad feelings about that @awight @thiemowmde?

Change 592279 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] [WIP] Add note on core conflict screen about new UI

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

we could hard code the current small set of wikis that have the feature as default and exclude them from showing this note. ( because there everyone was forced to see the new feature and could opt out of it )

I don't think there's a difference between the small default wikis and the remaining wikis, in this regard. Anyone who was opted out of the beta feature will be opted out again after we deploy T249659: Migrate beta feature opt-outs, even if they've already created a new preference. Also, I'd imagine the majority of users haven't seen an edit conflict since we deployed as small default, so they'll eventually have exactly the same experience on these wikis as on the others.

Anyone who was opted out of the beta feature will be opted out again after we deploy {T249659]

Ok especially in that case it then would make no sense to treat them differently. I agree. Thanks for the clarification.

After a lot of discussion with Sarai about the design system (since at the moment dismissable messages are not there), here is where we landed.

Mock with JS


Use OOUI close icon, size S 14x14 px, color: Base30. Align top with with top of info icon (should be 12 px from top) and 24px from the right.

No-JS
After doing some research, Sarai found examples of modals using only CSS. Does this mean it might be possible to do the above version without JS? Wanted to better understand if it's possible but a lot of work or not possible.

If we cannot use the same design for both, then here are the options. @Lena_WMDE your input would be great here. Neither is really ideal.

  1. The design is the same but without the "X" and the message is not dismissable. I understand that the message may be annoying to users who have opted out actively, but it is a neutral, unintrusive message which is temporary.
  1. Use a checkbox to set cookie. I've figured out a design for this that is ok, but it's not great. I've gotten the go ahead from Sarai, but we agreed to use it only if the other options were a clear no first.
WMDE-Fisch added a comment.EditedApr 28 2020, 8:46 AM

Hey cool, looks good so far. :-)

The modal idea is really nice and could work. The only issue here is that we do not only want to close the box. We also want to save the fact that the user does not want to see that box on future conflicts. Without JS this can only be done if there's some input submitted with the form that can then be used to store that decision. ( e.g using a checkbox )

It might be possible to combine the modal idea with a checkbox hidden behind the x that will be checked when the user dismisses the dialogue. I can try that and I'll keep you updated.

Closing the box without JavaScript is possible:

<input type="checkbox" id="mw-twocolconflict-dismiss-new-info">
<div class="mw-twocolconflict-new-info">
<label for="mw-twocolconflict-dismiss-new-info">X icon goes here</label>
<p>Text goes here.
</div>

<style>
#mw-twocolconflict-dismiss-new-info {
	display: none;
}
.mw-twocolconflict-new-info {
	border: 1px solid #000;
}
#mw-twocolconflict-dismiss-new-info:checked + .mw-twocolconflict-new-info {
	display: none;
}
</style>

This even contains the "never show again" checkbox as a hidden element. We will receive it's state in the backend and can store it.

That's great news. So the plan is to use the same design for JS and no-JS, but it will work a bit differently in the background?

WMDE-Fisch changed the point value for this task from 3 to 2.Apr 29 2020, 8:16 AM

@ecohen When I use 14x14px for the icon it looks like this, that seems way to tiny

If I try to implement the look of the mock I'm more on the default icon size of 20x20 and that's the same size as the info icon of the message itself. How should I proceed? :-)

@WMDE-Fisch, yea, the close icon intentionally does have a padding that makes it look like it's smaller, even if it's the default size of 20x20. I assume this is where the confusion came from. Please use the default size.

Actually, I put 14x14 because the design system says that this is the default size for icons on desktop. So maybe there is some confusion there about the actual default? I'll pass this on to Sarai but am fine in this case using the 20x20 with padding that you mention Thiemo.

Thanks @WMDE-Fisch for bringing up the clarity issue. After discussing with Lena, we've decided that it's ok to leave the message as is (no wording changes, no tooltip).

If a user is going to read it, they will likely only look at it the first time. If they want to change the setting they will, if not they'll close it. If they don't read it, then having it come back is not very helpful. In this case, the setting for it to not come back after dismissal is being polite more than anything. I think it's also the expected behavior for dismissing a message.

Change 592279 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add note on core conflict screen about new UI

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

Final design as of patch set 5 of https://gerrit.wikimedia.org/r/592279:

Remarks:

  • Note the x icon in the mock was gray, but is black now. That's partly because we can't easily change the color of OOUI icons, and because we aren't really supposed to fiddle with these colors, if I remember correctly. A workaround might be possible, getting us close, but not exactly to Base30. @ecohen, please tell us if we should do this, or if the default color is ok.
  • I noticed it's impossible to ever get this message back after it was closed once. It's stored in the users settings. There is no way (other that manually calling the API, e.g. via Special:ApiSandbox).
  • I still wonder if the x icon should have a tooltip because it dismisses the message forever, and I find this surprising. E.g. "Never show again".

I would like to propose a minor change in behavior: Whenever a user enables the new UI, we should kill the preference that was hiding the message in the old UI. This does have two effects:

  • It kills a useless preference.
  • The message will be shown again when a user enabled and disabled the new UI.

Change 594109 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Fix spelling and capitalization mistakes

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

Change 594117 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Fix minor merge issue in SplitConflictUtils::mergeTextLines

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

Change 594109 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Fix spelling and capitalization mistakes

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

awight added a comment.May 4 2020, 9:52 AM

I would like to propose a minor change in behavior: Whenever a user enables the new UI, we should kill the preference that was hiding the message in the old UI.

Oh, I like that!

If it would require a workaround, then let's leave the 'X' icon as black. I'd like to know more about the issue though to share with Sarai re: the design system. That's where I was taking the colors from.

I'll let @Lena_WMDE respond to the behavior change proposal and make the final call on the tooltip. I still feel the same as above. But I just wanted to clarify your suggestion. Do you mean that the message "Never show again" would show when the user hovers over the X or are you proposing adding a blue dot/ guider?

No, just when hovering the X, as a normal in-browser tooltip.

About the colors: In the code, base30 is documented as WCAG 2.0 level AA at 4.52:1 contrast ratio on #fff. This made me wonder, since the background behind the X is not white, but Base80. I play around with https://webaim.org/resources/contrastchecker/ and found:

  • base30 on base80 even fails AA for "normal text", which is probably not good.
  • base20 passes AA.
  • base10 passes AAA. But that's so close to base0 (black) that it's probably not worth fiddling with, especially since we need to go with a workaround (opacity) anyway.

Thanks @thiemowmde for flagging this! I would say we:

  • Keep the X the black color dictated by OOUI and avoid implementing a work around
  • Add the in-browser tooltip to the X saying "Never show again" as you suggest, and
  • Leave the behaviour as it is. On the plus side this change cleans up user preferences, but I think it's likely a low impact change in terms of UX. If a user enables and then disables the preference, I would assume they've done this because they don't want the new UI so seeing the message will not change their decision again (and maybe they'll even be annoyed about seeing the message again). Every user in the old UI will see this message at least once, and as long as we migrate the beta preferences before or at the same time as we activate the message even those users who's preferences have been changed because of the beta preference migration will see the message once and be able to switch back to the new UI if they wish.

Change 594140 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Add "never show again" tooltip to core UI message

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

Change 594140 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add "never show again" tooltip to core UI message

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

If a user […] disables the preference, I would assume they've done this because they don't want the new UI so seeing the message will not change their decision again […]

Thanks! This makes a lot of sense.

Krinkle added a subscriber: Krinkle.EditedMay 5 2020, 2:52 AM

Change 592279 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add note on core conflict screen about new UI

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

I noticed that there are a number of additional bundles registered on all page views as of this week, two of which are from this commit.

It looks like the added code is only a few bytes in size. The registry meta-data, loading instructions, and module wrappers for these bytes most likely cost about as much as the module itself, which make it several orders of magnitudes smaller than the kind of code I would recommend registring new entry point bundles for.

Please consider adding to other JS/CSS payloads instead, even if only conditionally utilised.

See also Page load, Module registry, and Init perf budget.

@Krinkle, thanks a lot for always having an eye on these numbers! That's super helpful. Unfortunately I don't fully understand how this is related. I hope you are able to provide a bit more information.

Yes, the main patch in this ticket introduced 2 new modules. But these are not loaded "on all page views", but conditionally only when a conflict occurs, see https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/TwoColConflict/+/592279/5/includes/TwoColConflictHooks.php.

When I look at the Grafana board and filter for TwoColConflict, I see only 1 additional module, not 2. The number jumps from 7 to 8 at 2020-05-04 approx. 16:35. I don't understand how this is related to a code change. As far as I can tell the numbers are from the English Wikipedia live site. The last deployment was done days ago, 2020-04-30. I'm very much willing to do a detailed bisect, but would love to understand the data we have much better before. Any help is very much appreciated!

Change 594409 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Merge core CSS into generic CSS RecourceLoader module

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

Change 594419 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Minimize ….CoreHint.js module a bit

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

Change 594432 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Merge ….CoreHint.js module into the ….init.js module

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

Change 594448 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] Add basic test for onEditPageShowEditFormFields

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

Change 594419 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Minimize ….CoreHint.js module a bit

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

Change 594503 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] Add browser test for the core ui hint

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

Change 594448 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add basic test for onEditPageShowEditFormFields

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

Change 594432 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Merge ….CoreHint.js module into the ….init.js module

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

Change 594409 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Merge core CSS into generic CSS RecourceLoader module

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

Krinkle removed a subscriber: Krinkle.May 11 2020, 9:40 PM

Change 594503 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add browser test for the core ui hint

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

WMDE-Fisch removed WMDE-Fisch as the assignee of this task.May 12 2020, 11:47 AM
WMDE-Fisch moved this task from Demo to Sprint Backlog on the WMDE-QWERTY-Sprint-2020-04-29 board.
WMDE-Fisch added a subscriber: WMDE-Fisch.

While Demo-ing we decided, that the link in the message to the preferences should also open in a new tab.

I had a quick look, and here are a few ideas I tried:

  • I tried to introduce an OpenInNewTabMessageLocalizer that works as a facade around an actual MessageLocalizer, but returns parse results where all the links open in new tabs. Unfortunately I got stuck, because this interface doesn't return strings, but Message objects. That's not an interface. Introducing a Message subclass would be an extreme Technical-Debt I personally don't want.
  • It's indeed possible to get a fresh parser (literally Parser::getFreshParser()) and use it with custom ParserOptions with $parserOptions->setExternalLinkTarget( '_blank' ) being set. However:
    • This only works for external links, which means we must make sure we use the [//www.mediawiki.org/… …] syntax in all messages, as well as in all translations.
    • I really don't want to inject a parser into the places where this is needed. This would definitely need a tiny service class that hides the parser.
    • Even then I'm not sure it's even possible to get access to the parser, without using MediaWikiServices::getInstance()->getParser(). Maybe it's fine to just use that.
    • The Technical-Debt is enormous.

Maybe it's fine to move the preg_replace() call we already have to a static function in SplitConflictUtils.

Whatever we do, please keep SplitConflictUtils trivial and to not use any MediaWikiServices there.

Change 594117 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Fix minor merge issue in SplitConflictUtils::mergeTextLines

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

Change 596164 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] Test core hint stays hidden after first dismissal

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

WMDE-Fisch moved this task from Sprint Backlog to Doing on the WMDE-QWERTY-Sprint-2020-04-29 board.
WMDE-Fisch changed the point value for this task from 2 to 1.

Change 596169 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] Open preference link in core hint in new tab

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

Change 596169 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Open preference link in core hint in new tab

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

Change 596164 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Test core hint stays hidden after first dismissal

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

thiemowmde closed this task as Resolved.May 13 2020, 11:57 AM
Lena_WMDE moved this task from Done to Demo on the WMDE-QWERTY-Sprint-2020-04-29 board.