Skip to content

Commit

Permalink
Visual optimisation for the CSV import process
Browse files Browse the repository at this point in the history 
When importing multiple CSV files at once, remove each entry from the
list of CSV files as its import completes. This way people can see the
list shrink visibly onscreen.

Also don't close the window if there are still files left to be
imported. This allows the user to import unchecked files, too, probably
using different settings.

See issue #1072.
  • Loading branch information
@MKleusberg
MKleusberg committed Sep 19, 2017
1 parent 969d3e4 commit 8f82f26
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 8 deletions.
27 changes: 23 additions & 4 deletions src/ImportCsvDialog.cpp
View file
Expand Up @@ -171,18 +171,37 @@ void ImportCsvDialog::accept()
// Get all the selected files and start the import
if (ui->filePickerBlock->isVisible())
{
bool filesLeft = false;

// Loop through all the rows in the file picker list
for (int i = 0; i < ui->filePicker->count(); i++) {
auto item = ui->filePicker->item(i);
if (item->checkState() == Qt::Checked) {
int row = ui->filePicker->row(item);

// Check for files that aren't hidden (=imported) yet but that are checked and thus marked for import
if (item->checkState() == Qt::Checked && !ui->filePicker->isRowHidden(row)) {
importCsv(item->data(Qt::DisplayRole).toString(), item->data(Qt::UserRole).toString());

// Hide each row after it's done
ui->filePicker->setRowHidden(row, true);
} else if(!ui->filePicker->isRowHidden(row)) {
// Check for files that aren't hidden yet but that aren't checked either. These are files that are still left
// to be imported
filesLeft = true;
}
}
}
else
{

// Don't close the window if there are still files left to be imported
if(filesLeft)
{
QApplication::restoreOverrideCursor(); // restore original cursor
return;
}
} else {
importCsv(csvFilenames.first());
}

QMessageBox::information(this, QApplication::applicationName(), tr("Import completed"));
QApplication::restoreOverrideCursor(); // restore original cursor
QDialog::accept();
}
Expand Down
7 changes: 3 additions & 4 deletions src/MainWindow.cpp
View file
Expand Up @@ -1174,12 +1174,11 @@ void MainWindow::importTableFromCSV()
validFiles.append(file);
}

if (!validFiles.isEmpty()) {
if (!validFiles.isEmpty())
{
ImportCsvDialog dialog(validFiles, &db, this);
if (dialog.exec()) {
if (dialog.exec())
populateTable();
QMessageBox::information(this, QApplication::applicationName(), tr("Import completed"));
}
}
}

Expand Down

6 comments on commit 8f82f26

@justinclift
Copy link
Member

Choose a reason for hiding this comment

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

Ahhh, the Travis failure on this is a real failure, not one of the recent Travis infrastructure problem ones:

[ 57%] Building CXX object CMakeFiles/sqlitebrowser.dir/src/MainWindow.cpp.o

/home/travis/build/sqlitebrowser/sqlitebrowser/src/MainWindow.cpp: In member function ‘void MainWindow::doubleClickTable(const QModelIndex&)’:

/home/travis/build/sqlitebrowser/sqlitebrowser/src/MainWindow.cpp:858:59: error: ‘currentlyBrowsedTableName’ was not declared in this scope

             (db.getObjectByName(currentlyBrowsedTableName())->type() == sqlb::Object::Types::Table);

                                                           ^

/home/travis/build/sqlitebrowser/sqlitebrowser/src/MainWindow.cpp: In member function ‘void MainWindow::dataTableSelectionChanged(const QModelIndex&)’:

/home/travis/build/sqlitebrowser/sqlitebrowser/src/MainWindow.cpp:882:59: error: ‘currentlyBrowsedTableName’ was not declared in this scope

             (db.getObjectByName(currentlyBrowsedTableName())->type() == sqlb::Object::Types::Table);

                                                           ^

make[2]: *** [CMakeFiles/sqlitebrowser.dir/src/MainWindow.cpp.o] Error 1

make[1]: *** [CMakeFiles/sqlitebrowser.dir/all] Error 2

make: *** [all] Error 2

@MKleusberg
Copy link
Member Author

Choose a reason for hiding this comment

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

Oh ok. Looks like I screwed something up during the backport process. This commit itself should be fine. I'll look into it in a second.

@justinclift
Copy link
Member

Choose a reason for hiding this comment

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

Heh Heh Heh, it's not the backport commit, it's the visual CSV one. 😄

@MKleusberg
Copy link
Member Author

Choose a reason for hiding this comment

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

But it does seem to depend on the branch. If I click the red Travis icon for this commit in the master branch, I get the "10 minutes without output" message which looks like a Travis issue to me. If I click the red Travis icon for this commit in the v3.10.x branch, I get the error message you've mentioned above.

@MKleusberg
Copy link
Member Author

Choose a reason for hiding this comment

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

Let's see if this fixes it 😄

@justinclift
Copy link
Member

Choose a reason for hiding this comment

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

Oops, I didn't check the branch name when looking at Travis. Yep, you're definitely right. 😄

Please sign in to comment.