| awight | |
| Feb 17 2020, 10:37 AM |
| F31628864: Screenshot from 2020-02-24 11-53-39.png | |
| Feb 24 2020, 10:56 AM |
| F31625491: image.png | |
| Feb 21 2020, 1:07 PM |
| F31625476: image.png | |
| Feb 21 2020, 1:07 PM |
| F31625403: Screenshot from 2020-02-21 12-43-12.png | |
| Feb 21 2020, 11:47 AM |
| F31625405: Screenshot from 2020-02-21 12-43-05.png | |
| Feb 21 2020, 11:47 AM |
| F31623912: image.png | |
| Feb 20 2020, 12:10 PM |
| F31623910: image.png | |
| Feb 20 2020, 12:10 PM |
| F31623903: image.png | |
| Feb 20 2020, 12:06 PM |
We currently default to the "other" revision for each conflicting chunk, because the radio buttons load with the checked attribute set on the left side. Get rid of this default, we want the page to load with neither column selected.
We're leaving the non-JS behavior for a later ticket.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | WMDE-Fisch | T245410 Neither column of a conflict should be selected by default | |||
| Stalled | None | T245505 Non-JS error handling when a conflict is submitted with no column selected | |||
| Stalled | None | T248573 Allow Non-JS validation by re-populating the form | |||
| Resolved | WMDE-Fisch | T246870 Non-JS conflict resolution might ignore reverts in specific cases | |||
| Resolved | WMDE-Fisch | T246781 Non-JS conflict resolution might forget about edits made in the UI | |||
| Resolved | thiemowmde | T245736 Show dynamic label text depending on conflict column selection | |||
| Resolved | WMDE-Fisch | T246646 Add missing browser tests for "nothing selected by default" behavior |
@Hanna_Petruschat_WMDE What should we do if the form is submitted before the user has selected a side for each two-column chunk? Should we show an error dialog? Should we draw a red error box around the incomplete radio groups?
Change 572842 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] [WIP] Default to not selecting either side
We already designed a version for that when we planned the whole new UI. Here is a mock up
The flow should be:
A user enters the resolution page, then does not select one version in a conflicting paragraph. When hitting "Save Changes" the screen should jump to the not selected version – if there are multiple not selected ones, to the one which is the highest up on the page. There should also be a warning below the Save button as shown in the mock.
@awight Feel free to hit me up, if there is more to be defined.
Note: edit pencils and text box highlighting should also be unselected, however first see decisions made in T209702: Edit icon in new two column edit conflict design appears active when it can't be edited.
Thanks a lot for sharing the prototype @Hanna_Petruschat_WMDE I am also at the office today can help if there are further iterations needed.
@Hanna_Petruschat_WMDE @Erdinc_Ciftci_WMDE I have another design question, showing both columns translucent really takes away from the diff coloring. In general, fading the diff coloring seems problematic because there's so much less visual contrast, and the use case involves identifying changed text and copying, so we know people will want to see diffs.
Here's what it looks like now,
Note to self, I'm going to tweak the row div so that it includes the "Please select a version" text, which currently comes before the row. Layout should be unchanged when there's no error, but when the error is present, 5px or so margins will appear on every side.
Change 573328 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] [WIP] Validate that a selection is made for every chunk
Change 573547 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] [WIP] Put selection label inside of the row
Change 573555 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Error style for conflict rows
@Hanna_Petruschat_WMDE @Erdinc_Ciftci_WMDE
Minor layout question. For technical reasons, I found it was easier to pull the label down into the row. That happens to match the prototype better, but changes rendering. Please share your opinions:
With errors:
Another divergence from the mockup: the drawing has a "Please select a version" line for each row, and apparently dynamic text for "Your version" and "Other version" depending on the selection, but we only display once at the top.
Change 573328 abandoned by Awight:
[WIP] Validate that a selection is made for every chunk
Reason:
squashed
Change 573570 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Handle conflict column without a selection
Change 573571 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] [DNM] Open the conflict form with no columns selected
Everything is a no-op until the final one-line patch, which is marked "DNM". Actually unselecting the radio buttons by default depends on server-side validation for safety.
... but we only display once at the top.
@awight Why do we only show it once? Because we are talking about non-js at some point (and dynamic text is not possible there)? As far as I got it this ticket is related to the js use case, isn't it?
It will be a necessary text to be shown in place otherwise the user is confronted with a red box and no call to action to follow.
It would be fine for me if you'd pull the label down into the row.
That makes sense, thanks! I'll be sure to show the text in case of an error. I'll also split the dynamic text question into a new task: T245736: Show dynamic label text depending on conflict column selection
Pulling back into "Doing" to restore this label text.
@Hanna_Petruschat_WMDE, the patch https://gerrit.wikimedia.org/r/573547 includes a design change. Personally, I'm not sure if I like it. Independent from how I feel, I wonder how it should look like from the UX's team perspective?
As of now:
What the patch proposes:
Note that this change does make the middle column wider, leaving less room for the yellow and blue boxes left and right.
Unfortunately the mock here in this ticket contains both versions. Which one should we use?
Change 573555 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Error style for conflict rows
In my last conversation with her, Hanna said it basically doesn't matter. If we see further problems we can adjust, of course.
The functional difference in this design change is that every row includes a description, not only the first line. It's worth noting that if we choose the "above" layout, following rows' labels should presumably use the same layout.
Mobile layout should be considered as well. The elements flow awkwardly, even as is:
The mocks in the tickets show both options in terms of something above and all the text in between but the middle column is always the same width. I understand the concern of having a wider middle column but this will also allow for potential longer words in other languages without needing hyphenates.
I think it is definitely worht looking at mobile. We haven't taken a closer look into that so far... But it's also not our main use case.
@Hanna_Petruschat_WMDE, here is a screenshot of how it currently looks. I can not spot a difference to the mock.
One minor question though: Do you think we should use rounded corners for the red rectangles? The OOUI style guide suggests to use very subtle (2px only) rounded corners, I believe.
Nice!! Thanks Thiemo. And yes, you're eagle eyes are absolutely correct about the 2px radius. The box shall have it too
Change 574418 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Add OOUI conforming border-radius to red error box
Change 573547 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Put selection label inside of the row
Change 573570 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Handle conflict column without a selection
Change 572842 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Validate that all chunks have a selection
Change 574418 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add OOUI conforming border-radius to red error box
Change 573571 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Make sure a side is selected in all browser tests
Change 574799 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Don't pre-select a side, and full non-JS validation support
Change 576006 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Show "please select a side" on all rows, not only the first
Change 576010 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Browser tests: Check if no side is selected by default
Change 576006 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Show "please select a side" on all rows, not only the first
Change 576671 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] Don't pre-select a column when JS is used
Change 576671 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Don't pre-select a column when JS is used
Change 576010 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Browser tests: Check if no side is selected by default