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

core, add confirmation dialog on exit #208

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

amitsagtani97
Copy link
Contributor

Patch for Task5013

--> Shows confirmation dialog on exiting an activity or closing an activity.

@@ -47,6 +47,9 @@ ActivityBase {
id: activity
focus: true
activityInfo: ActivityInfoTree.rootMenu
// set to true when returned to home from a dialog.
property bool returnFromDialog: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This variable is needed to handle the cases when we close a dialog.
home() is called when we close a dialog, so to prevent the confirmation dialog to pop up, in that case, I have used this variable to handle this.

Copy link
Member

@petitlapin petitlapin left a comment

Choose a reason for hiding this comment

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

It misses a configuration variable to enable this feature.
If you click multiple times on Escape, the dialog will pop up as much times as the number of clicks.

else {
Core.showMessageDialog(parent,
qsTr("Do you really want to quit?"),
qsTr("Yes"), function() { pageView.pop();
Copy link
Member

Choose a reason for hiding this comment

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

go back to line for the function if it is not a one line function

Copy link
Member

Choose a reason for hiding this comment

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

this code is copied/pasted 3 times, can be in a function instead

// A confirmation dialog to quit the current activity is popped.
else {
Core.showMessageDialog(parent,
qsTr("Do you really want to quit?"),
Copy link
Member

Choose a reason for hiding this comment

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

"Do you want to leave current activity?" would be better

@@ -47,6 +47,9 @@ ActivityBase {
id: activity
focus: true
activityInfo: ActivityInfoTree.rootMenu
// set to true when returned to home from a dialog.
property bool returnFromDialog: false

onBack: {
Copy link
Member

Choose a reason for hiding this comment

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

I think it should also be added there (I don't have mouse with back button but I think it will trigger this signal)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This signal is triggered only on back pressed from phones or other touch devices. Currently, back button from the mouse is not enabled.

Copy link
Member

@petitlapin petitlapin left a comment

Choose a reason for hiding this comment

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

When I leave GCompris I have the following error:
qrc:/gcompris/src/core/GCDialog.qml:131: TypeError: Cannot read property of null
qrc:/gcompris/src/core/GCDialog.qml:132: TypeError: Cannot read property of null
qrc:/gcompris/src/core/GCButtonCancel.qml:33: TypeError: Cannot read property of null
qrc:/gcompris/src/core/GCButtonCancel.qml:34: TypeError: Cannot read property of null
qrc:/gcompris/src/core/GCDialog.qml:185: TypeError: Cannot read property of null
qrc:/gcompris/src/core/GCDialog.qml:204: TypeError: Cannot read property of null
file:///usr/lib/qt/qml/QtQuick/Controls/Private/Control.qml:90: ReferenceError: parent is not defined
file:///usr/lib/qt/qml/QtQuick/Controls/Private/BasicButton.qml:194: ReferenceError: parent is not defined
file:///usr/lib/qt/qml/QtQuick/Controls/Private/Control.qml:90: ReferenceError: parent is not defined
file:///usr/lib/qt/qml/QtQuick/Controls/Private/BasicButton.qml:194: ReferenceError: parent is not defined

Can you check?

When I disable the option, I still have the confirmation on linux.

The option should be disabled by default.

Can you rebase on master and add the corresponding unit test?

@@ -213,6 +213,15 @@ Item {
}
}

GCDialogCheckBox {
id: enableExitDialogBox
text: qsTr("Enable activity Exit confirmation dialog")
Copy link
Member

Choose a reason for hiding this comment

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

Confirm before leaving current activity

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.

2 participants