Fix text formatting QML and enable/disable #31647
Conversation
elpaso
added
the
Bug
|
@nyalldawson I'm not totally sure about the fix in case of DD properties for the background, mind having a look? |
|
#31428 is deliberate, in that we also need these properties to be editable if data defined enable buffer is used. |
|
|
||
| if ( textStyleElem.firstChildElement( QStringLiteral( "dd_properties" ) ).isNull() ) | ||
| QDomElement ddElem = textStyleElem.firstChildElement( QStringLiteral( "dd_properties" ) ); | ||
| if ( ddElem.isNull() ) |
There was a problem hiding this comment.
Hmm - this doesn't look right. We'd still want to restore these even when data defined properties are present. What's the logic here?
There was a problem hiding this comment.
properties are present. What's the logic here?
I said it wasn't clear to me :)
The question is what is the current (broken) logic ?
In case of missing DD and issue #31427, the QML is read twice:
First time here:
https://github.com/qgis/QGIS/blob/master/src/core/qgstextrenderer.cpp#L1934
which gets the correct config
and second time here:
https://github.com/qgis/QGIS/blob/master/src/core/qgstextrenderer.cpp#L1939
which gets the wrong config
The DD getter also seems to be duplicated:
here is checked intially https://github.com/qgis/QGIS/blob/master/src/core/qgstextrenderer.cpp#L1937
and here is checked again: https://github.com/qgis/QGIS/blob/master/src/core/qgstextrenderer.cpp#L1946
I moved the getter before to simplify the code.
There was a problem hiding this comment.
ok - got an alternate fix coming through. Seems it was a copy/paste error on my behalf originally.
Ok, I've committed some more complex logic to handle frame enabled but I have some doubts about the UX/UI: is the the DD is an override of the main checkbox? Anyway I think we should make this clearer to the user. |
elpaso
force-pushed
the
text-formatting-bugs
branch
from
9e9173e
to
193a9db
Compare
Fixes #31428 and #31427