QField icon indicating copy to clipboard operation
QField copied to clipboard

Add the possibility to rotate a single feature (line or polygon)

Open qsavoye opened this issue 1 year ago • 1 comments

add_rotate_feature

qsavoye avatar Oct 15 '24 14:10 qsavoye

I just remove some submodules depency that I do not had to commit previously, sorry for the incovenient.

qsavoye avatar Oct 17 '24 07:10 qsavoye

@qsavoye , hey there, thanks for this contribution, it's fantastic! I'll get to review it ASAP (I've been a bit under the weather this week).

One thing I can already see as a need here is to change the icon to use a stock one from Google's material symbols & icons library: https://fonts.google.com/icons?icon.query=rotate -- that'll insure the style matches the rest of the app.

nirvn avatar Oct 25 '24 09:10 nirvn

@qsavoye , oh, one question: you are aware that the processing toolbox (new feature in QField 3.4) has a rotate algorithm. Is there a reason why you wouldn't be using that instead?

nirvn avatar Oct 25 '24 09:10 nirvn

@qsavoye , oh, one question: you are aware that the processing toolbox (new feature in QField 3.4) has a rotate algorithm. Is there a reason why you wouldn't be using that instead?

@nirvn It's more a question of user experience. I tried the processing toolbox for rotation on my Iphone, the parameter window hides a big part of the map canvas, which can be unpleasant Furthermore, in my opinion, it is faster to rotate with drag or mouse pointer on mapcanvas than with a precise angle in degrees. I think both are complementary.

qsavoye avatar Oct 25 '24 12:10 qsavoye

@qsavoye , thanks for addressing the review. Will merge as soon as the CIs turn green. Thanks for this contribution :pray:

nirvn avatar Nov 19 '24 09:11 nirvn

@nirvn I think my there was an update between the current master and and the master where i start these pull request on file multifeaturelistmodelbase.cpp

in canRotateSelection function

if ( !vlayer || vlayer->readOnly() || !( vlayer->dataProvider()->capabilities() & QgsVectorDataProvider::ChangeGeometries ) || vlayer->customProperty( QStringLiteral( "QFieldSync/is_geometry_locked" ), false ).toBool() )

must be replace to

if ( !vlayer || vlayer->readOnly() || !( vlayer->dataProvider()->capabilities() & Qgis::VectorProviderCapability::ChangeGeometries ) || vlayer->customProperty( QStringLiteral( "QFieldSync/is_geometry_locked" ), false ).toBool() )

Do you see other change which can block the merge ?

qsavoye avatar Nov 19 '24 09:11 qsavoye

@qsavoye , no, if that's fixed, I think we can merge.

There was indeed a QGIS core library update which did change a couple of enum keys, inc. this one.

nirvn avatar Nov 19 '24 12:11 nirvn

@qsavoye , can you resolve the conflict too?

nirvn avatar Nov 19 '24 12:11 nirvn

@qsavoye , merging this before it gets into a conflict state again. Thanks for this contribution :pray:

nirvn avatar Nov 27 '24 07:11 nirvn