Remove expression evaluation in layout legend widget #56372
Conversation
|
@elpaso I did not take the time to answer your review, sorry. So I just rebased on master, and I can't reopen #53631.
No, it's only for the layout, and specifically with atlas enabled.
I did not, because it appeared to be too custom here (and I chose the "minimal" effort way, when I realized that a big refactoring would be the right approach...). |
python/PyQt6/core/auto_generated/layertree/qgslayertreemodellegendnode.sip.in
Show resolved
Hide resolved
|
@elpaso Seems ok to you? |
|
Unrelated check error |
|
Thanks @elpaso 👍🏼 😃 |
|
Still unrelated test failure |
Djedouas
force-pushed
the
fix-53442
branch
2 times, most recently
from
bb56f25
to
fcaad72
Compare
|
I did a light rebase to categorize the changes, to avoid having too many commits. |
|
Again, unrelated test error |
Re-opening of #53631 for bugfix session.
Description
This PR comes from trying to fix #53442.
I jumped into parts of code that are very strange.
The evaluation of expressions for the
QgsLayoutLegendItemin the composer, vs the evaluation of expressions in theQTreeViewin the composer legend widget are somewhat correlated but not completely...I will explain my observations, and why I suggest the changes in this PR.
User observations
The layout item always evaluates the legend label, expression or not being present (in
QgsLayerTreeModelLegendNode::draw)The legend tree view does not always evaluate the legend labels
The legend tree view does not evaluate expressions when refreshing the layout (
) or when moving to the next atlas page (except if the filtering for items - in linked map or atlas - is enabled)
Technical observations
Technically, what is very strange, is that the label evaluation occurs in the
getDatamethod of the legend model.That means that the model is changed inside a method that should not change the model (and the model is not aware of this change).
Also, the legend layout item evaluates the label of the model, whereas the legend tree view evaluates the
UserLabel(edit role) to change and update the model label...The way it is implemented is also strange (evaluating children symbols nodes but not layer nodes, only if the layer node has an expression... !).
After a lot of time digging into the code, trying to update the labels at the right place/moment, I ended with this conclusion:
the expression evaluation is very situational (symbol expressions that can be added for every child, atlas page number, layer feature-count enabled, user-defined variables, filters, etc.) and should not be part of the model. It is a representation, by definition, the real data being the raw expression. Furthermore, the expressions are today evaluated at different places with possibly different contexts (for the legend layout item and for the legend tree view).
Suggested changes
I think that the model should contain the label, with the textual expression (not evaluated), and that it is the responsibility of the view to evaluate the expression, like it is done with the legend layout item.
As a result, in this PR I removed the label evaluation taking place in the
getDatamethod of the model.The proposition is to always see the expressions in the legend tree view, and leave the layout legend item be the only one evaluating the label.
It is technically easier because evaluating the label in every situation is difficult to handle (with expressions being able to be in the layer style label, in the legend node widget, with the entity count, atlas page, etc.), so having the "total" expression, not evaluated, in the model label makes sense, and it is evaluated at the very end, for the rendering.
From a user point of view it can be argued that it is also more convenient to see his expression in the legend tree widget, and the evaluation on his composer page.