Page MenuHomePhabricator

Convert RadioRangeBallot to use OOUI
Closed, ResolvedPublic


As part of T273044: Convert SecurePoll VotePage to use OOUI, this included ballot will need to return an OOUI object

Event Timeline

I'll go ahead and do the more aggressive switch to getInputOOUI in since I think you'd want to do it here anyways.

EDIT: actually, it's a lot more work, and I think it's workable without it. So... maybe not?

Change 670369 had a related patch set uploaded (by STran; owner: STran):
[mediawiki/extensions/SecurePoll@master] Use OOUI for RadioRangeBallot.php

HTMLForm has a "flatlist" modifier which lines up check/radio inputs inline. This modifier doesn't seem to be ported into OOUI. I think this came up during some earlier ports to OOUI but it wasn't as obvious. If I have too many options or questions in a radio range ballot, it ends up looking fairly unwieldy:

image.png (604Γ—279 px, 19 KB)

And here's what it used to be:

image.png (184Γ—365 px, 8 KB)

@Prtksxna @Tchanders @DLynch I've got a few questions:

  • Did I understand the limitation correctly? I didn't see any modifier in the docs/skimming the code
  • Even if the radio buttons are inline, I don't think there's any way to get that table look using a FieldLayout. Can someone confirm?

Given this, what do we want to do about it, assuming it's a non-zero cost to do something? SecurePoll CSS? Add inlining upstream?

Using something like a ButtonSelectWidget might reduce the vertical space the radios are taking up. Would that be possible on a page like this?

Had another look at this and chatted with @DLynch and I don't think ButtonSelectWidget is the correct way to go because the markup uses buttons. This 1. removes the semantic relationship between the inputs (radio inputs are all tied together by their name attribute) and 2. makes it difficult to support no-js/use standard form submits since buttons don't normally pass along data like that*

  • We do pass along some data with the submit button, but I don't think this is necessarily supported out of the box

I have a few other possible proposals, ranked from easiest to hardest (with a somewhat linear correlation of Less Wrong to More Right)

  1. Do nothing - literally nothing. We just leave it as is. The OOUI stuff is being deprioritized as the quarter's ending so we could just not get to whatever's left, including this task.
  2. Use CSS to style the radio buttons inline - I think this works, as long as there isn't any ltr/rtl stuff to consider
  3. Use OOUI to re-create the table structure - I think this is doable because you get to define the name for each radio input so it would be a bunch of radio inputs with the same name and one option each
  4. Make CheckMatrixWidget’s compatible with RadioInputWidget - theoretically doable, based on the same assumption that makes 3 doable
  5. Make a MatrixWidget that only provides the scaffolding for the matrix and allows you to pass whatever objects into it, including check inputs and radio inputs

Moving this to Needs Design because it depends on what sort of final layout we're okay with, assuming we're moving forward. 3-5 would get us closer to the original layout but 2 is faster if we're okay with duplicating the labels like that. We don't necessarily even have to do any of this yet, hence 1.

My primary concern is that the basic layout of the voting interface remains the same, and it doesn't take up more vertical space than it needs to (especially as the number of candidates can easily be over 10). So, I'd be against (1).

Would (2) look something like this?

Screenshot 2021-03-11 at 6.21.36 PM.png (354Γ—1 px, 47 KB)

If so, this is workable, as its the same vertical space. But not ideal because of the repetition.

If 3-5 would essentially be the same as the current layout I would prefer that. This is one of the few user facing interfaces of the feature and I think it'd be good to keep it the same.

Change 670847 had a related patch set uploaded (by DLynch; owner: DLynch):
[mediawiki/extensions/SecurePoll@master] RadioRangeBallot to use OOUI widgets

That patch is option 3, above. It winds up being really-equivalent to the current design, just with the OOUI radio widgets in the table.


image.png (324Γ—570 px, 19 KB)

Patch does:

image.png (320Γ—626 px, 21 KB)

Change 670369 abandoned by STran:
[mediawiki/extensions/SecurePoll@master] Use OOUI for RadioRangeBallot.php


@Prtksxna bumping this back into design review so you can say whether you're okay with this.

@Prtksxna bumping this back into design review so you can say whether you're okay with this.

This is looking good! Thanks @DLynch and @STran ✨
(should I move this to QA or Done?)

Back to code review until someone approves the patch, I guess.

Change 670847 merged by jenkins-bot:
[mediawiki/extensions/SecurePoll@master] RadioRangeBallot to use OOUI widgets

dom_walden added a subscriber: dom_walden.

I more-or-less repeated the testing I did for T276901#6928421 for elections of type "Range voting (plurality)" and "Range voting (histogram range)".

Test Environment: SecurePoll 2.0.0 (97412a0) 07:26, 19 March 2021.

Change 676086 had a related patch set uploaded (by DLynch; author: DLynch):

[mediawiki/extensions/SecurePoll@master] RadioRangeBallot was the only Ballot not returning a FieldsetLayout

Change 676086 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@master] RadioRangeBallot was the only Ballot not returning a FieldsetLayout

Niharika triaged this task as Medium priority.