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

CommentFileSizeAuthor
#5 aggregator-2072249-5.patch30.89 KBtim.plunkett
#1 aggregator-2072249-1.patch35.35 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Component: field_ui.module » aggregator.module
Status: Active » Needs review
StatusFileSize
new aggregator-2072249-1.patch35.35 KB

The fact that aggregator categories aren't entities is REALLY confusing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

This is simple swap for t to $this->t so I think it is safe to RTBC.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Title: Modernize forms in aggregator.module » Modernize aggregator.module forms

Standardizing issue names

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Patch no longer applies

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
StatusFileSize
new aggregator-2072249-5.patch30.89 KB

#1957330: Make possible for parsers and fetchers to expose configuration through plugins took care of most of SettingsForm, here's the rest.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Add link to meta issue