Page MenuHomePhabricator

NumberInputWidget is not actually an InputWidget
Closed, ResolvedPublic

Description

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

Also, validation should be optional.

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 added a subscriber: Krenair.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptJan 27 2016, 2:54 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

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 triaged this task as Normal 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
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 19 2016, 10:07 PM

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

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

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

Volker_E moved this task from Doing to Next-up on the OOUI board.

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

@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/ (I94e35de4dbd92d76605895b8b559ca5d25b72819) 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.

@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/ (I94e35de4dbd92d76605895b8b559ca5d25b72819) 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.

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.