issue #61146, behavoir of the Reset value button in Inspector #2011
Conversation
Jojo-Schmitz
force-pushed
the
reset-value
branch
2 times, most recently
from
29ac4cd
to
483b9f1
Compare
Jojo-Schmitz
changed the title
| @@ -236,7 +236,7 @@ QVariant TempoText::propertyDefault(P_ID id) const | |||
| { | |||
| switch(id) { | |||
| case P_ID::TEMPO: return 2.0; | |||
| case P_ID::TEMPO_FOLLOW_TEXT: return false; | |||
| case P_ID::TEMPO_FOLLOW_TEXT: return true; | |||
There was a problem hiding this comment.
Sorry for the delay to come back to this, but I don't get why we need to change the default for this property. Also, changing it here but not in the constructor doesn't look elegant. Is it really needed to change the default? Why?
There was a problem hiding this comment.
because if you take a tempo text from the palette, it has the 'reset value' button enabled. I think TEMPO_FOLLOW_TEXT being set to true is a sane default. Setting it in constructor too though causes many test failures and you (rightfully) complained about those ;-)
Also this seems to be a backward compatibility thing.
There was a problem hiding this comment.
For properties, the default behavior is to not write them to MSCZ files if they aren't default. Apparently for this one, we don't use writeProperty and we will write it only if "true". Then, when we read, it's set to false in the constructor, and read from the file to be set to true if necessary.
If we change the default to true here, we loose the opportunity to call write/read property for this property :-(
So I'd vote for keeping default to false, and use writeProperty/readProperty as it should be. It's more symmetric.
From a user perspective, it changes nothing since tempo from palette or via Create > Text > Tempo are created with the followtext. The only thing, it's not the default so reset is enabled. Not a problem to me.
There was a problem hiding this comment.
Hmm, we could uncomment the tt.resetFollowText->setDisabled(followText); in InspectorTempText::postInit() to solve the reset button being enabled on those tempo texts.
Well, it does only partly, toggling 'follow text' also toggles the reset button's availabilitly, but using the latter doesn't change the former, which is as bad as not solving it...
Where (and how) to use those writeProperty/readProperty? Or would that be in a separate change anyway?
Dafault for "follow text" remains false, so the 'reset value' button remains enabled tor tempo texts taken from the palette. Still the tempo setting is used, even if 'Follow Text' is enabled and the that text says something entirely different.
issue #61146, behavoir of the Reset value button in Inspector
No description provided.