Page MenuHomePhabricator

Accept and reject buttons are too similar
Open, NormalPublic

Description

The new reject button that shows up for reviewers is too easy to confuse with the accept button (especially when one is patrolling a lot of revisions in a short time). There should be an obvious visual clue to tell them apart, like green/red color.


Version: unspecified
Severity: enhancement

Details

Reference
bz26256

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 21 2014, 11:17 PM
bzimport set Reference to bz26256.
bzimport added a subscriber: Unknown Object (MLST).
Tgr created this task.Dec 5 2010, 10:24 PM
aaron added a comment.Dec 5 2010, 11:36 PM

I had a coloring scheme, but it was removed (but not replaced with anything).

We did intend to replace it with something (e.g. icons on the buttons), but didn't have time for the proper solution. I felt pretty strongly that the current all-black, all-the-time button text was better than the button scheme, which I think Brandon concurred with me on (though I'm not positive I checked wiht him). Anyway, it's good we have an issue logged to make sure we do this right. Assigning to Brandon for a quick mockup when he gets the chance.

Jorm added a comment.Dec 6 2010, 6:10 PM

The reject button shouldn't be a button if we can avoid it. It should just be a text link.

MaxSem added a comment.Dec 6 2010, 7:38 PM

(In reply to comment #3)

The reject button shouldn't be a button if we can avoid it. It should just be
a text link.

Then accept shouldn't be a button too for consistency.

Jorm added a comment.Dec 6 2010, 7:40 PM

Not necessarily.

To be honest, the entire accept/reject/unaccept interface should have a totally different UI. But the resources to make it so aren't there right now (and won't be committed unless we have further sign off).

I suppose the question really is this: "is it possible to undo a 'reject'" and if the answer is no or 'yes but very difficult or tedious' then we should downplay reject. We know we can always revert an accept.

Tgr added a comment.Dec 6 2010, 9:17 PM

(In reply to comment #5)

I suppose the question really is this: "is it possible to undo a 'reject'" and
if the answer is no or 'yes but very difficult or tedious' then we should
downplay reject. We know we can always revert an accept.

It is difficult (and pollutes page history), but as long as you need two clicks for the reject, that isn't much of a problem. Mistaking the options (or even just having to think about which one to choose) wastes patrollers' time, though.

demon removed a subscriber: demon.Dec 16 2014, 7:57 PM

@Ladsgroup Maybe something you would want to look into…

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptMar 16 2017, 12:23 AM
Tgr added a comment.Mar 16 2017, 12:31 AM

FWIW this is the current interface:

Tgr added a comment.Mar 16 2017, 12:34 AM

I should probably just switch it to WikimediaUI. I don't think it existed when I filed this bug.

Although it's a form on top of a diff page, so people will be sensitive about form height,

I think we can simply change it to OOUI which automatically handles this task as well. I looked at the codebase and it doesn't look hard. I will get to this in the weekend.

Made https://gerrit.wikimedia.org/r/344820 (gerrit-bot doesn't add comment when I make a patch)

Before:


After:

The highet is not that different.

The buttons probably don't need to be primary?

Ladsgroup added a comment.EditedMar 26 2017, 2:03 PM

Hmm, I'm not sure. It looks like primary to me because it does a major job (accepting or rejecting a set of revisions). But it's not definitely my call. @Volker_E, thoughts?

Quiet buttons for comparison:

Tgr added a comment.Mar 26 2017, 5:42 PM

Making both of them quiet would defeat the purpose of this bug. Making accept normal and reject quiet would be an option, but IMO the primary makes sense - when you are a reviewer and looking at an unreviewed diff, you are expected to do something about it.

Would a primary accept and a quiet reject work?

The reject button is as the important as the accept button, even more.

@Ladsgroup If there's not one clearly indicated primary(!) action, but two, equally important, OOjs UI developers invented the concept of the quiet progressive and destructive buttons side-by-side.
The strict rule around the primary button is that it should be applied on only one button per view.
At https://en.wikipedia.org/w/index.php?title=Special:UserLogin we've specifically styled mediawiki.UI to allow for a non-primary, progressive button. I'd suggest going that way here as well.

Side-note, in your latest screenshot, the label for the textbox seems to have lost the margin in-between.

This is how it looks like after adding quiet flags:

I don't like it.

Tgr added a comment.May 2 2017, 5:59 PM

https://en.wikipedia.org/w/index.php?title=Special:UserLogin we've specifically styled mediawiki.UI to allow for a non-primary, progressive button. I'd suggest going that way here as well.

mw-ui-primary doesn't seem to do anything. The login page has a non-primary non-progressive (but also not quiet) button for the signup link.

My comment was obviously misleading, we styled the signup button there, not mediawiki.UI in general. I'm thinking of something like

Well, that doesn't exist in mediawiki.ui (as you mentioned) and I don't think we can migrate the review form to OOUI without refactoring half of the extension. I don't have any better option now.

@Ladsgroup Why not specifically style those buttons accordingly in extension's CSS?

Some screenshots of the current version of the patch. (These are with the default configuration, things look different on each of our production wikis.)

BeforeAfter
Page view
Diff view

Issues:

  • The space between the comment field and its label disappeared.
  • On page view, the comment field is not styled.

Also, the "Unaccept revision" button (shown when viewing an accepted revision) is inconsistent now:

In order to provide users a better experience, resulting in higher acceptance in that change it would need some extra per-extension CSS:

  • don't provide two primary action buttons, rather a solution as proposed in T28256#3234020
  • adding margin-bottom: .5em` to .mw-fr-ratingselects || .fr-ratingoptions
  • vertical align the “Comment:” label

This is after manual CSS injection:

BeforeAfter

Now I added a little bit of margin-top so It doesn't stick to upper elements.

I couldn't get to do anything about this one. Hopefully I will tackle it in Wikimania hackathon.