improve GUI consistency & UX for data-defined style (fixes #9881) #1833

vmora · 2015-01-14T17:57:05Z

No description provided.

nyalldawson · 2015-01-14T20:07:57Z

vmora: such a great change to hide behind such a humble commit message! ;) A few quick notes from some initial tests of the feature (I haven't checked the code changes):

tab order for symbol widgets needs to be updated
expressions/field selections are lost if the data defined button is set to not active and then the dialog is closed. While this was also the previous behaviour, it differs from how data defined buttons behave in labelling & composer, and is frustrating to use. This should be addressed with this PR so that the expression/field is saved along with the active status of the button, so that data defined properties can be easily disabled and then re-enabled.

luca76 · 2015-01-14T20:36:11Z

This commit is for....?

brushtyler · 2015-01-14T22:03:50Z

src/gui/symbology-ng/qgssymbollayerv2widget.cpp

+  QGS_BLOCK_SIGNAL_DURING_CALL( spinOffset, setValue( mLayer->offset() ) )
+  QGS_BLOCK_SIGNAL_DURING_CALL( cboPenStyle, setPenStyle( mLayer->penStyle() ) )
+  QGS_BLOCK_SIGNAL_DURING_CALL( cboJoinStyle, setPenJoinStyle( mLayer->penJoinStyle() ) )
+  QGS_BLOCK_SIGNAL_DURING_CALL( cboCapStyle, setPenCapStyle( mLayer->penCapStyle() ) )


The previous 3 calls may introduce a bug as they doesn't runs in the same order than the old code.

Here's the old behaviour:
3 blockSignals(true)
then 3 calls
then 3 blockSignals(false).

Please check it.

My reasoning was that, since no signal is emitted, it is equivalent.

I understand that if one of the widget has a pointer on one of the others, a signal may be fired by the pointer-to widget. I checked in the custom widgets of qgis and I did not find a widget which holds a pointer on another one.

If you thing my reasoning is wrong, I can restore the old behavior, since it's not directly related to the feature.

brushtyler · 2015-01-14T22:21:51Z

src/gui/symbology-ng/qgssymbollayerv2widget.cpp

+  QGS_BLOCK_SIGNAL_DURING_CALL( mOutlineWidthUnitWidget, setUnit( mLayer->outlineWidthUnit() ) )
+  QGS_BLOCK_SIGNAL_DURING_CALL( mOutlineWidthUnitWidget, setMapUnitScale( mLayer->outlineWidthMapUnitScale() ) )
+  QGS_BLOCK_SIGNAL_DURING_CALL( mHorizontalAnchorComboBox, setCurrentIndex( mLayer->horizontalAnchorPoint() ) )
+  QGS_BLOCK_SIGNAL_DURING_CALL( mVerticalAnchorComboBox, setCurrentIndex( mLayer->verticalAnchorPoint() ) )


The same as above, please check if the above 2 lines behave like the old code.

NathanW2 · 2015-01-14T23:11:38Z

@vmora nice work. Bringing some consistency here will be great.

@dakcarto you did the label version. Do did you want to run a eye over this version too?

vmora · 2015-01-15T09:18:53Z

Hi all,

Sorry for the title (and commit message), I'll rebase and force update I my repository when the other issues with this PR are fixed.

tab order for symbol widgets needs to be updated

@nyalldawson I don't understand what you mean by that. Can you explain a bit more plz.

expressions/field selections are lost if the data defined button is set to not active and then the dialog is closed. While this was also the previous behaviour, it differs from how data defined buttons behave in labelling & composer, and is frustrating to use. This should be addressed with this PR so that the expression/field is saved along with the active status of the button, so that data defined properties can be easily disabled and then re-enabled.

@nyalldawson I think the right thing to do is to use à QMap< QString, QgscDataDefined* > instead of a QMap< QString, QgscDataDefined* > for mDataDefinedProperties of QgsSymbolLayerV2. But this is a new feature, and touches other parts of the code. I'd prefer to do that as a second independent PR (incremental improvement) to avoid running into conflicts with this PR, I think it's more 'agile' :).

This PR tries to be specific to one issue, I will also work on the diagram afterward to do the same kind of things, and then on the 'Advanced' menu. I can do the change you request in between, or after that, and have a specific PR for that.

nyalldawson · 2015-01-15T09:23:21Z

@vmora - fair enough. Its been a pet peeve of mine for a long time so I was hoping this work would address it.

Re tab order - the changes to the ui files have resulted in a messed up tab order for the symbol dialogs. You can see it if you try and tab through the widgets in any symbol layer properties window.

vmora · 2015-01-15T09:53:55Z

@nyalldawson ok, I see the tab order problem now. thanks, I'm on it.

nyalldawson · 2015-01-16T00:05:51Z

@vmora tab order looks good to me now, thanks. (the exception is that map unit widgets are always last in the order, but that issue isn't new).

NathanW2 · 2015-01-19T21:54:40Z

@dakcarto @wonder-sk any thoughts on this. It would be good to merge for 2.8.

nyalldawson · 2015-01-20T00:16:51Z

src/gui/symbology-ng/qgsellipsesymbollayerv2widget.cpp

+
+  mDataDefinedPropertyButtons.clear();
+
+#define QGS_REGISTER_DATA_DEFINED_BUTTON( button, name, type, description )\


@vmora I don't think this should be a macro - I can't see any benefit in implementing it this way, and it breaks things like Qt Creator's find/follow/etc. I'd prefer that this was a function in QgsSymbolLayerV2Widget.

vmora · 2015-01-20T11:30:59Z

@nyalldawson thanks for the suggestions, this greatly improves the code, see #1842

I left out the QGS_BLOCK_SIGNAL_DURING_CALL, since this was just added to help me read the code and is unrelated.

Closing the PR since #1842 is a replacement.

fix 9881

d69ce2a

brushtyler reviewed Jan 14, 2015
View reviewed changes

NathanW2 changed the title ~~fix 9881~~ mprove GUI consistency & UX for data-defined style (fix #9881) Jan 14, 2015

brushtyler reviewed Jan 14, 2015
View reviewed changes

jef-n changed the title ~~mprove GUI consistency & UX for data-defined style (fix #9881)~~ improve GUI consistency & UX for data-defined style (fixes #9881) Jan 14, 2015

fix tab order

05f3544

nyalldawson reviewed Jan 20, 2015
View reviewed changes

vmora mentioned this pull request Jan 20, 2015

improve GUI consistency & UX for data-defined style #1842

Closed

vmora closed this Jan 20, 2015

vmora deleted the issue9881 branch August 14, 2015 07:25

qgib mentioned this pull request May 24, 2019

Improve GUI consistency & UX for data-defined style #18402

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve GUI consistency & UX for data-defined style (fixes #9881) #1833

improve GUI consistency & UX for data-defined style (fixes #9881) #1833

vmora commented Jan 14, 2015

nyalldawson commented Jan 14, 2015

luca76 commented Jan 14, 2015

brushtyler Jan 14, 2015

vmora Jan 15, 2015

brushtyler Jan 14, 2015

NathanW2 commented Jan 14, 2015

vmora commented Jan 15, 2015

nyalldawson commented Jan 15, 2015

vmora commented Jan 15, 2015

nyalldawson commented Jan 16, 2015

NathanW2 commented Jan 19, 2015

nyalldawson Jan 20, 2015

vmora commented Jan 20, 2015


		mDataDefinedPropertyButtons.clear();

		#define QGS_REGISTER_DATA_DEFINED_BUTTON( button, name, type, description )\

improve GUI consistency & UX for data-defined style (fixes #9881) #1833

improve GUI consistency & UX for data-defined style (fixes #9881) #1833

Conversation

vmora commented Jan 14, 2015

nyalldawson commented Jan 14, 2015

luca76 commented Jan 14, 2015

brushtyler Jan 14, 2015

Choose a reason for hiding this comment

vmora Jan 15, 2015

Choose a reason for hiding this comment

brushtyler Jan 14, 2015

Choose a reason for hiding this comment

NathanW2 commented Jan 14, 2015

vmora commented Jan 15, 2015

nyalldawson commented Jan 15, 2015

vmora commented Jan 15, 2015

nyalldawson commented Jan 16, 2015

NathanW2 commented Jan 19, 2015

nyalldawson Jan 20, 2015

Choose a reason for hiding this comment

vmora commented Jan 20, 2015