Part of #2072251: [meta] Modernize forms to use FormBase

Updated: Comment #N

Problem/Motivation

Now that #2059245: Add a FormBase class containing useful methods is in, I was looking at old forms that have empty validateForm() methods, or still use Drupal::service() or t().

Proposed resolution

Convert existing FormInterface forms to extend FormBase

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

RainbowArray’s picture

Is this something we should check with [attiks] on to get this change started?

RoSk0’s picture

Status: Active » Needs review
FileSize
1.5 KB

Initial patch.

plopesc’s picture

Status: Needs review » Needs work

It looks good to me.

However you didn't included in your patch changes that should be made in PictureMappingFormController class. Could you work on this class too?

Thank you!!

RoSk0’s picture

Status: Needs work » Needs review
FileSize
4.33 KB
5.84 KB

Good catch.
New patch.

RoSk0’s picture

Assigned: Unassigned » RoSk0
RainbowArray’s picture

Assigned: RoSk0 » Unassigned
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs reroll

Patch no longer applies. Needs reroll, then review.

RoSk0’s picture

RoSk0’s picture

Status: Needs work » Needs review

Sorry for the buzz

Sutharsan’s picture

Issue tags: -Needs reroll

Needs reroll tag removed.

attiks’s picture

Issue tags: +Needs reroll

FYI: picture is renamed to responsive_image so this needs a reroll once #2124377-74: Rename "Picture" module to "Responsive Image" module is committed

JayeshSolanki’s picture

Eli-T’s picture

Component: picture.module » responsive_image.module
JayeshSolanki’s picture

Title: Modernize picture.module forms » Modernize responsive_image.module forms
attiks’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, thanks

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit 6832239 on 8.x by webchick:
    Issue #2078001 by RoSk0, JayeshSolanki | alexanansi: Modernize...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.