Closed mgarciavaadin closed 1 year ago
While reviewing the PR for this issue, @sissbruecker raised a concern regarding the second signature of the method to be added. I would like to invite the people involved in the decision making process for this feature to voice their opinions on his suggestion so we can decide whether to follow it or not.
@bennewi @rolfsmeds @mshabarov @mgarciavaadin @gtzluis
All I know is that we, during the meeting where this was discussed, were unsure how the renderer and the valueprovider work together in the Flow Grid, thus the following remark in the planning document:
Todo - verify the interaction between the two in V8 and see if it fits in Flow
What was the result of that research?
The framework version of the method does:
Sets the Renderer for this Column. Setting the renderer will cause all currently available row data to be recreated and sent to the client.
The presentation provider is a method that takes the value of this column on a single row, and maps that to a value that the renderer accepts. This feature can be used for storing a complex value in a column for editing, but providing a simplified presentation for the user when not editing.
The method that is introduced for the Flow component in the linked PR does not do the same. It does not map from the row type to a type that the renderer accepts. Instead it sets the value provider as comparator, which is only used for sorting, and the renderer just takes the row type as usual.
It's not clear how this would help with migrations, it seems the methods do rather different things.
Describe your motivation
This new method would have an impact on the modernization of Vaadin 8 to Flow 24 applications where the setRenderer(Renderer<? super V> renderer) and/or setRenderer(ValueProvider<V,P> presentationProvider, Renderer<? super P> renderer) were frequently used. Also for new developments it would provide a way to set or change the renderer after the call to Grid.addColumn specially when the signature addColumn(String propertyName) without a Renderer is used.
Describe the solution you'd like
New Grid.Column methods available:
Describe alternatives you've considered
The approach seems to be straightforward so no alternatives have been considered.
Additional context
Not sure if it will work out of the box, because the current implementation assumes setup on first roundtrip. Further testing is required.