Problem/Motivation

DateTimeDefaultWidget contains a todo to this issue

Proposed resolution

Resolve @todo

Original report by @dawehner

#2034563: Switch field plugin managers to DefaultPluginManager

For all the logic needed in the current container factory maybe the createInstance method on the widget plugin manager should be different.

CommentFileSizeAuthor
#14 datetimedefaultwidget-2035317-14.patch2.23 KBhussainweb
#10 2035317-10.patch2.11 KBandypost
#5 2035317-5.patch804 bytesandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

yched CreditAttribution: yched commented

There was once a FormatterFactory too, which IIRC @tim.plunkett removed in the same patch that introduced the @FieldFormatter plugin annotation.
So maybe similarly this could be done in #2035315: Add a dedicated @FieldWidget annotation ?

yched’s picture

yched CreditAttribution: yched commented
Issue tags: +Field API

tagging

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue tags: +Needs issue summary update

is this issue still valid?

yched’s picture

yched CreditAttribution: yched commented
Status: Active » Closed (duplicate)

Nope, this was done in #2035315: Add a dedicated @FieldWidget annotation

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue summary: View changes
Status: Closed (duplicate) » Needs review
FileSize
2035317-5.patch804 bytes

There's @todo left

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Shouldn't we actually inject it then instead of just removing the @todo? Also, that @todo just says inject once this issue is in, so this issue was a blocker for that but there's no reason to do re-open this issue?

Status: Needs review » Needs work

The last submitted patch, 5: 2035317-5.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

@berdir so file new issue? suppose there's no reason to inject EM here, ince there's a method

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs work » Needs review

5: 2035317-5.patch queued for re-testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Title: Switch widget factory to container factory » DateTimeDefaultWidget should implement ContainerFactoryPluginInterface
Issue summary: View changes
FileSize
2035317-10.patch2.11 KB

Proper title and summary -we need to resolve @todo

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue tags: -Needs issue summary update +Quick fix
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs review » Reviewed & tested by the community

Still think this should have been a different issue, I'm sure there are other formatters and widgets that use global services too that are simply missing a corresponding @todo.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 2035317-10.patch, failed testing.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
FileSize
datetimedefaultwidget-2035317-14.patch2.23 KB

I am rerolling the patch. There were conflicts from #2005434: Let 3rd party modules store extra configuration in EntityDisplay.

@Berdir, I will see if I can find more such widgets and create issues.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Status: Needs work » Needs review
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Perfect

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed c679460 and pushed to 8.x. Thanks!

  • alexpott committed c679460 on 8.x 
    Issue #2035317 by andypost, hussainweb | dawehner: DateTimeDefaultWidget...

Status: Fixed » Closed (fixed)

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