Patch

[Rishubh Jain]

If he is quitting then there can be 2 possible things....either the person clicked there by mistake or he dont want to make changes...and whatever options you have given there i have just put the options in the message box...its one and the same thing..just the user dont have to go back there to save or cancel...why do you want the user to go back??...i have discussed this with Tomaz before doing this




On Saturday, 8 March 2014 10:27 AM, Dirk Hohndel <dirk at hohndel.org> wrote:
 
On Sat, 2014-03-08 at 12:52 +0800, Rishubh Jain wrote:
> Do I need to redo something...or will this patch be accepted?

Rishubh, if you look through my comment you'll see that I believe that
your patch is wrong - at the point in the flow where you inserted it we
DON'T want to offer those options. We really do want to send the user
back to either cancel or save her changes to the dive she is editing.

But even if I didn't disagree with the patch itself, I would have asked
you to redo it because of the indentation issues and because of the
issues with the commit message.


/D


> 
> On Saturday, 8 March 2014 10:15 AM, Dirk Hohndel <dirk at hohndel.org>
> wrote:
> 
> On Sat, 2014-03-08 at 12:24 +0800, Rishubh Jain wrote:
> > I hope this is it...The patch is just an enhancement . its a small
> try
> > to improve the UI of subsurface. While Quiting instead of asking the
> > user to go back and either save or discard changes it asks the user
> to
> > save changes or discard the changes there and there to save his/her
> > effort to go back and to make subsurface more user friendly
> 
> This is an interesting idea. But you misunderstand what the code in
> question does. When this code is run the user is actively editing a
> dive. They need to go back and either cancel or save that edit.
> 
> For regular "exit" from Subsurface we already give users the option to
> not exit, exit without saving, or to save the file.
> 
> Also, some feedback on the patch.
> 
> > From 777d890a28b9204afab60e4d5d63879c9d8d92be Mon Sep 17 00:00:00
> 2001
> > From: Rishubh <rishubh at Rishubh.(none)>
> > Date: Sat, 8 Mar 2014 09:47:55 +0530
> > Subject: [PATCH] ADDED: More interactive dialogue box When user
> quits the
> >  subsurface he/she will get a message to save, discard
> >  changes or to cancel the quitting option. Signed-off-by:
> 
> >  Rishubh <
> rishubh at Rishubh.(none)>
> 
> 
> As you can tell, the commit message is all run together into the
> Subject
> line including the SOB. When editing the commit message please make
> sure
> that you have one empty line after the commit title and another empty
> line before the SOB.
> 
> > ---
> >  qt-ui/mainwindow.cpp |  25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> > 
> > diff --git a/qt-ui/mainwindow.cpp b/qt-ui/mainwindow.cpp
> > index 20f463c..288d480 100644
> > --- a/qt-ui/mainwindow.cpp
> > +++ b/qt-ui/mainwindow.cpp
> > @@ -635,9 +635,22 @@ void MainWindow::closeEvent(QCloseEvent *event)
> >  {
> >      if (DivePlannerPointsModel::instance()->currentMode() !=
> DivePlannerPointsModel::NOTHING ||
> >          ui.InfoWidget->isEditing()) {
> > -        QMessageBox::warning(this, tr("Warning"), tr("Please save
> or cancel the current dive edit before closing the file."));
> > -        event->ignore();
> > -        return;
> > +            QMessageBox::StandardButton reply;
> > +            reply = QMessageBox::question(this, "Warning", "Do you
> want to save the file before closing?",
> > +                                QMessageBox::Save|
> QMessageBox::Discard|QMessageBox::Cancel);
> > +            if (reply == QMessageBox::Save) {
> > +              file_save();
> > +              qDebug()<<"Your file has been saved";
> > +              QApplication::quit();
> > +            }            
> > +            else  if(reply==QMessageBox::Discard){
> > +              event->accept();
> > +              QApplication::quit();
> > +            }
> > +            else if(reply==QMessageBox::Cancel){
> > +              qDebug()<<"cancel";
> > +              event->ignore();
> > +            }
> 
> Here your whitespace is wrong... You have one too many levels of
> indentation.
> 
> Please pay attention to these details :-)
> 
> /D
> 
> 
> 
> 
> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20140308/50291e8f/attachment.html>