Page MenuHomePhabricator

Implement HTMLFormFieldCloner in OOUIHTMLForm
Closed, ResolvedPublic

Description

Implement HTMLFormFieldCloner in OOUIHTMLForm

Event Timeline

Change 421373 had a related patch set uploaded (by Mainframe98; owner: Mainframe98):
[mediawiki/core@master] Add OOUI for HTMLFormFieldCloner

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

Change 421373 abandoned by Mainframe98:
Add OOUI for HTMLFormFieldCloner

Reason:
Unable to work on this now

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

Volker_E removed a project: Patch-For-Review.
Volker_E moved this task from Reviewing to Backlog on the OOUI board.

Change 421373 restored by Mainframe98:
Add OOUI for HTMLFormFieldCloner

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

Change 421373 merged by jenkins-bot:
[mediawiki/core@master] Add OOUI for HTMLFormFieldCloner

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

matmarex removed a project: Patch-For-Review.
matmarex added a subscriber: matmarex.

Known issue with the patch:

There is also one problem with the functionality: all of the fields added in JS are generated with identical "id" attributes, so clicking their labels will focus the first one, rather than the correct one. This is a bit hard to explain, so here's a video:


(I used this code for testing, based on the one you gave me: https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/463121)

I'm not sure how to fix this. You might be able to do it with some clever replacing in cloner.js (like with the 'uniqueId'), but that seems very unreliable.

The proper way to do this would be to not use HTML "templates" for OOUI widgets, but instead construct new widgets from the config options. But at a glance, that would require a lot of refactoring in HTMLFormField (there's no way to get the config options out of it). :/

In my opinion this is not a severe enough problem to block the patch from being merged (although it may be severe enough to discourage you from using HTMLFormFieldCloner in your OOUIHTMLForm).