Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contextmenu with individual actions #5961

Merged
merged 1 commit into from
Jan 8, 2018
Merged

Conversation

signedav
Copy link
Contributor

Individual actions in the contextmenu of the attributetable and the relationeditorwidget.

It's unlink feature and delete feature in the relationeditorwidget.
It's delete feature in the attributetable (form view)

Description

There are several action types like the maplayer actions and the user definded actions. These are actions that are specific for the GUI we are using. They do only appear there.

It's unlink and delete feature in the relationeditorwidget.
contextmenu_relationeditorwidget
(here you can see the duplicate feature maplayer actions too)
contextmenu_attributetable
It's delete feature in the attributetable (form view)

Unlike the maplayer actions these actions are not available when not in the edit mode. It can be discussed how it should be.

@nyalldawson
Copy link
Collaborator

What's the empty entry at the top of the menu for?

@signedav
Copy link
Contributor Author

signedav commented Jan 2, 2018

@nyalldawson It's an empty user defined action I used for tests and forgot to remove when I made the GIFS. Wasn't paying attention anymore on that myself.
It's actually like this (without the MapLayerActions):
contextmenu gif

@nyalldawson
Copy link
Collaborator

Ok, that makes sense :) wasn't sure if it was a design decision here

@@ -1042,6 +1043,21 @@ void QgsAttributeTableDialog::setFilterExpression( const QString &filterString,
}


void QgsAttributeTableDialog::deleteFeature( const QgsFeatureId &fid )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter should be passed by value QgsFeatureId fid, this is actually a simple long long which is faster to pass by value than by ref.

mLayer->deleteFeature( fid );
}

void QgsAttributeTableDialog::showContextMenu( QgsActionMenu *menu, const QgsFeatureId &fid )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass QgsFeatureId by value

{
QgsFeatureIds featureids;

if ( featureid )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks dangerous, the featureid 0 is valid.

void deleteFeatures( const QgsFeatureIds &featureids );

/**
* Unlinks the features
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"providing an empty list will use the current selection."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed that. There are three methods now per function: duplicateSelectedFeatures (called by button), duplicateFeature (called by contextmenu), where duplicateFeature calls the duplicateFeatures with a list containing only one current feature and the duplicateSelectedFeatures with a list containing the selected features. It's not depending on the size of the FeatureIds if it's selected or not...

@signedav signedav force-pushed the work_contextmenu branch 2 times, most recently from 4488327 to ec3fa09 Compare January 8, 2018 16:55
Individual actions in the contextmenu of the attributetable and the relationeditorwidget.
It's unlink and delete feature in the relationeditorwidget.
It's delete feature in the attributetable (form view)
@m-kuhn m-kuhn merged commit 8392d7f into qgis:master Jan 8, 2018
@nyalldawson
Copy link
Collaborator

@m-kuhn @signedav I'm seeing a consistent crash on qgis exit after the merge of these recent action related PRs.

@signedav
Copy link
Contributor Author

I'll see to it @nyalldawson

@signedav
Copy link
Contributor Author

PR for fix pending #6040

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants