Page MenuHomePhabricator
Create Task
Maniphest T124856

NumberInputWidget is not actually an InputWidget
Closed, ResolvedPublic
Actions

Description

It has various incompatibilities, such as setValue not being chainable and having no focus method.

Also, validation should be optional.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
NumberInputWidget: Remake as an actual TextInputWidget childoojs/uimaster+144 -82
NumberInputWidget: Extend InputWidget instead of Widgetoojs/uimaster+22 -21
Customize query in gerrit

Related Objects
Search...

Event Timeline

Krenair created this task.Jan 27 2016, 2:54 AM
Krenair raised the priority of this task from to Needs Triage.
Krenair updated the task description. (Show Details)
Krenair added a project: OOUI.
Krenair subscribed.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptJan 27 2016, 2:54 AM
Krenair added a parent task: T124850: Create dedicated input methods for "number" transclusion parameters.Jan 27 2016, 2:55 AM
gerritbot subscribed.Feb 8 2016, 12:23 AM

Change 268375 had a related patch set uploaded (by Prtksxna):
OO.ui.NumberInputWidget: Extend InputWidget instead of Widget

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

gerritbot added a project: Patch-For-Review.Feb 8 2016, 12:23 AM
• Prtksxna subscribed.Feb 8 2016, 12:29 AM

Doing this is a bit tricky since the NumberInputWidget uses the TextInputWidget's validity methods. This makes it hard for it to use its own normal <input> element. I think apart from the hack above we have two options:

  • Make NumberInputWidget implement all the InputWidget methods but not actually extend it.
  • Create a Validity mixin that makes it easier for all widgets to add this functionality.
Jdforrester-WMF assigned this task to • Prtksxna.Mar 2 2016, 2:13 AM
Jdforrester-WMF triaged this task as Medium priority.
Jdforrester-WMF set Security to None.
Jdforrester-WMF moved this task from Backlog to Doing on the OOUI board.Mar 8 2016, 11:19 PM
Volker_E subscribed.Apr 12 2016, 6:48 PM
Volker_E created subtask T132488: Disabled NumberInputWidget takes scrolling (touchpad, scrollwheel) into account when it shouldn't.Apr 12 2016, 7:02 PM
Volker_E created subtask T132489: NumberInputWidget: Triage changing `type=text` to `type=number`.Apr 12 2016, 7:06 PM
Ricordisamoa subscribed.Apr 12 2016, 8:40 PM
• Prtksxna closed subtask T132488: Disabled NumberInputWidget takes scrolling (touchpad, scrollwheel) into account when it shouldn't as Resolved.Apr 13 2016, 3:20 PM
matmarex closed subtask T132489: NumberInputWidget: Triage changing `type=text` to `type=number` as Resolved.Apr 19 2016, 10:07 PM
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 19 2016, 10:07 PM
• Prtksxna added a parent task: T130434: Break up TextInputWidget.Jul 20 2016, 7:09 AM
gerritbot added a comment.Jul 20 2016, 7:11 AM

Change 268375 abandoned by Prtksxna:
NumberInputWidget: Extend InputWidget instead of Widget

Reason:
Will come back to this once I3d60fb and I94e35d are merged, and TextInputWidget is leaner.

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

• Prtksxna mentioned this in T149644: NumberInputWidget doesn't have explicit label.Jan 26 2017, 7:50 AM
Volker_E removed a project: Patch-For-Review.Jan 27 2017, 6:22 PM
Volker_E moved this task from Doing to Next-up on the OOUI board.
Volker_E mentioned this in T130434: Break up TextInputWidget.Feb 19 2017, 5:00 AM
gerritbot added a comment.Apr 19 2017, 11:47 PM

Change 349125 had a related patch set uploaded (by Mooeypoo):
[oojs/ui@master] NumberInputWidget: Remake as an actual TextInputWidget child

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

gerritbot added a project: Patch-For-Review.Apr 19 2017, 11:47 PM
Volker_E added a comment.Apr 20 2017, 1:38 AM

@Prtksxna Are there considerations from your side about the latest patch, that might not have been resolved as https://gerrit.wikimedia.org/r/#/c/284168/ (I94e35d) hasn't got merged yet?

Volker_E moved this task from Next-up to Reviewing on the OOUI board.Apr 20 2017, 2:10 AM
• Prtksxna removed • Prtksxna as the assignee of this task.Apr 23 2017, 2:36 PM
• Prtksxna assigned this task to Mooeypoo.Apr 24 2017, 7:17 AM
• Prtksxna added a subscriber: Mooeypoo.
In T124856#3196372, @Volker_E wrote:

@Prtksxna Are there considerations from your side about the latest patch, that might not have been resolved as https://gerrit.wikimedia.org/r/#/c/284168/ (I94e35d) hasn't got merged yet?

I think we're good. I was planning on making NumberInputWidget inherit from InputWidget in which case, I am not sure why I added that issue/patch as a blocker. @Mooeypoo's patch on the other hand makes it inherit from TextInputWidget, while this should not be a problem, it gives NumberInputWidget some unnecessary config params.

gerritbot added a comment.Apr 24 2017, 9:11 PM

Change 349125 merged by jenkins-bot:
[oojs/ui@master] NumberInputWidget: Remake as an actual TextInputWidget child

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

Volker_E closed this task as Resolved.Apr 24 2017, 9:34 PM
Volker_E moved this task from Reviewing to OOjs-UI-0.21.2 on the OOUI board.
Volker_E edited projects, added: OOUI (OOjs-UI-0.21.2); removed: OOUI.
Volker_E removed a project: Patch-For-Review.
• Prtksxna awarded a token.May 1 2017, 10:18 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits