Closed (outdated)
Project:
Drupal core
Version:
8.6.x-dev
Component:
entity system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
22 Nov 2012 at 17:24 UTC
Updated:
4 Nov 2018 at 16:02 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
yched commentedComment #2
yched commentedComment #3
yched commentedComment #4
yched commentedComment #6
yched commented#4: 1830868-32-EntityDisplay_0.patch queued for re-testing.
Comment #8
yched commentedComment #10
yched commentedComment #12
yched commentedStupid me.
Comment #14
yched commentedComment #16
yched commentedComment #18
yched commentedComment #20
yched commentedComment #22
yched commentedComment #24
yched commentedComment #26
yched commentedComment #28
yched commentedComment #30
yched commentedComment #31
yched commentedmethod renames
Comment #33
yched commentedComment #34
yched commentedComment #35
yched commentedComment #36
yched commentedComment #38
yched commented#36: entity_display-testbot-1848068-35.patch queued for re-testing.
Comment #40
yched commentedComment #42
yched commentedComment #43
yched commentedStuck ?
Comment #45
yched commentedComment #47
yched commentedComment #49
yched commentedComment #51
yched commentedComment #52
yched commentedComment #54
yched commentedComment #56
yched commentedComment #58
yched commentedComment #60
yched commentedComment #62
yched commentedComment #63
yched commentedComment #65
yched commentedComment #67
swentel commentedKickstarting again, we really really need this for field group, ds and let contrib go nuts! :)
Comment #69
swentel commentedShould fix the remaining tests.
Comment #71
swentel commentedThis should be green
Comment #73
swentel commented#45: entity_display-testbot-1848068-44.patch queued for re-testing.
Comment #74
swentel commented#71: display-handlers-testbot-1848068-71.patch queued for re-testing.
Comment #75
Stalski commentedHi, I am looking for the @todo interface in the patch.
Just a question, could it be there is a left over in the patch: there are two
abstract class DisplayComponentHandlerBasefiles. I think the "Drupal\entity" namespaced one is not used anymore?Comment #76
yched commentedEw - right, Drupal\entity\DisplayComponentHandlerBase.php is stale.
For the interface, I was just waiting to settle on the actual methods in the handlers.
Comment #77
swentel commentedJust keeping up with HEAD
Comment #79
swentel commentedOh right, custom blocks and datetime field have landed too
Comment #80
swentel commentedreviving, branch opened in sandbox : entity-display-components-1875974 (I kind of lost the old one apparently)
Had to merge a lot, got notices on standard install, want to give this a roll though.
Comment #82
swentel commentedFix discovery
Comment #84
swentel commentedMore obvious fixes
Comment #86
swentel commentedSeriously
Comment #88
swentel commentedComment #89
swentel commentedMessy merge with HEAD, let's see what the bot thinks.
Comment #91
effulgentsia commented#89: 1848068-89.patch queued for re-testing.
Comment #93
swentel commentedre-kickstarting (for the 4th time) on top of the new form modes patch, let's see how this works out.
- also - I switched branches - 1875974-entity-components
Comment #95
swentel commentedThis should have a lot more fixes - a little too much code duplication, need to clean that up at some point.
Comment #97
swentel commentedUse 'type' again instead of formatter
Comment #99
swentel commentedFix more tests, amateescu made the pluginmanager to modern times
Comment #101
amateescu commentedReverted some stuff and fixed the default config files.
Comment #102
amateescu commentedRenamed the 'widget' handler type back to 'field'.
Comment #104
amateescu commentedFixed all getComponent() and setComponent() calls.
Comment #106
swentel commentedMore fixes - should fix a lot of notices in the upgrade path
Comment #108
swentel commentedEven more fixes
Comment #110
aspilicious commentedHelping this forward, fixing some tests.
Comment #111
swentel commentedaspilicious, can you commit those fixes to the sandbox also ? 1875974-entity-components
Comment #113
aspilicious commentedI'm a good boy did that *before* posting this patch :D
Comment #114
swentel commentedNew patch against latest HEAD
Comment #116
swentel commentedMore fixes, getting close to green (again).
Comment #118
swentel commentedComment #120
swentel commentedFix tons of notices
Comment #121.0
(not verified) commentedupdate issue #
Comment #122
amateescu commentedStarted to work on this but I got very confused when I got to DisplayComponentHandlerBase::getRenderer() because we also have a getRenderer() method in EntityDisplayBase. Was the original intention to move it from the config entity class to the new plugin type (component handler)?
Posting just an interdiff for now, nothing committed anywhere :/
Comment #123
yched commentedI think the intention was:
- external world calls $display-getRenderer($field_name)
- internally the display does $this->handler($field_name)->getRendeter() (and keeps a static cache of the results)
Comment #124
andypostSuppose
$typeshould be optional forgetComponent(), we already store handler_type in display so 123 could workBut for
setComponent()we need to know the the type of component!Merged current HEAD and made some clean-up, all pushed to 1875974-entity-components-andypost
Comment #126
andypost..drop is moving
Comment #127.0
(not verified) commentedUpdated issue summary.
Comment #128
aspilicious commentedI could install and some of the field stuff worked, lets see
Comment #129
aspilicious commentedComment #131
aspilicious commentedComment #132
aspilicious commentedComment #134
aspilicious commentedNow without upgrade path tests
Comment #135
aspilicious commentedComment #137
aspilicious commentedShould fix most problems, I hope
Comment #139
aspilicious commentedMore fixes! Less notices!
Comment #141
aspilicious commentedLets try this one
Comment #142
aspilicious commentedLess notices on this one.
Comment #144
aspilicious commentedComment #145
aspilicious commentedComment #147
aspilicious commentedComment #148
aspilicious commentedComment #151
aspilicious commentedComment #152
aspilicious commentedFixed some minor issues
Comment #153
aspilicious commentedAnother reroll
Comment #156
aspilicious commentedComment #158
andypostNo configuration is prepared so a lot of tests fail
Comment #160
andypostAnother set of changes:
- fix for massageIn() that executed for non-existing field - needs some research
- field handler now injects needed services
- some doc blcok fixes
- added todos
Comment #162
andypostRenamed
massageOut() => onComponentGet()Decoupled
massageIn() => onComponentSet() and onComponentRemove()Added doc-blocks
Comment #164
aspilicious commentedIn the onComponentSet function you need to "change" the options and return the new options.
You're returning a new array overriding the past options.
==> test fails :)
Comment #165
andypostshould fix failed tests, Field handler needs better check for removed field
Comment #166
yched commentedNo time to look into the interdiffs right now, but +1 on the onXxx() method renames, I was thinking the same while brushing my teeth yesterday night :-)
As for "Handler plugins persisted and shared in the Manager / setContext()" :
Yeah, that was an attempt to avoid creating 2 (or more) handler plugin objects for each EntityDisplay (that is, for each bundle in an entity_view() / entity_view_multiple()). Instead, patch creates one instance of each "handler type" and reuses them for the whole request, injecting the proper context each time with setContext().
I'm not really happy with that either. I fully agree that it is not exactly intuitive, and also probably doesn't play really well with nested entity_view() calls (e.g an entity_ref formatter displaying the referenced entity). Might be a premature optimisation, but hard to tell the exact impact without actual benchmarks though - and entity_view() is definitely a very critical path...
The best route would probably be to remove it for now and switch to a more common pattern of "each EntityDisplay has a PluginBag of Handler plugins". Then benchmark and see if/how we can optimize from there.
Comment #168
andypostMerge after #1988612: Apply formatters and widgets to rendered entity base fields, starting with node.title
Comment #170
andypostLet's test with interface
Comment #171
andypostcheck merge
Comment #173
andypostfix errors
EDIT open again #2210177: Use getSetting() in comment formatter
Comment #175
andypostOne more
Comment #176
andypostUse core's pattern to get field definitions
Comment #179
andypostrender pipeline should change, this just a hack that not commited
Comment #181
swentel commentedComment #183
swentel commentedComment #184
swentel commentedComment #187
aspilicious commentedLets see...
Comment #194
amateescu commentedI think further work on this can happen in the main issue now :)