Page MenuHomePhabricator

HTMLForm: Refactor button code to consolidate
Closed, DeclinedPublic


Right now, HTMLForm::getButtons() looks at various instance properties to build the submit, reset and cancel buttons, and then it loops over $this->mButtons to build any buttons that might have been added by calling $form->addButton(). The logic for how and when to build the submit, reset and cancel buttons is then duplicated in each subclass (at least in OOUIHTMLForm, and soon in CodexHTMLForm).

Instead, we should refactor this so that:

  • The HTMLForm base class gathers the information for the built-in buttons (submit, reset, cancel) and expresses this as an array in the same format as $this->mButtons. This could be done e.g. by adding a protected protected getButtonData() method that builds this array, and returns that array merged with $this->mButtons.
  • Both the base implementation of HTMLForm::getButtons() and the subclasses' implementations of it look at this array and build the buttons it describes.

This way we don't duplicate any logic; the only thing that each subclass does separately is building the buttons, which they all need to do differently.

This task is related to T361032: HTMLForm: Remove unused reset button; they could be done in either order, but removing the reset button first is probably easier.

Event Timeline

I no longer think we should do this, since:

  • The submit button has type=submit and has flags, both of which are unsupported for addButtons()
  • The cancel button is not a button at all, it's a link
  • The reset button is going to be removed (T361032)

So that just leaves us with two buttons that are both very different from the kind of thing you can do with addButtons() (one because it's a primary+progressive submit button, the other because it's not even really a button).