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

Add multiple folder selection to view notes in all selected folders[WIP] #1463

Merged
merged 8 commits into from
Dec 2, 2019

Conversation

Waqar144
Copy link
Contributor

  • Initial commit.

#327

while (*it) {
// hide all notes that are not in selection
if (!noteIdList.contains((*it)->data(0, Qt::UserRole).toInt())) {
(*it)->setHidden(true);
Copy link
Owner

@pbek pbek Nov 27, 2019

Choose a reason for hiding this comment

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

You cannot simply show or hide notes in the list. Don't forget about everything MainWindow::filterNotes() does! 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really? 🤔
But it does hide notes, and shows the ones from the selected folders 🤔
I don't know the implications this will have for all other modules though and the side effects...

MainWindow::filterNotes() would require a lot of changes to work with multi folder selection, right? or maybe I didn't understand you

Copy link
Owner

Choose a reason for hiding this comment

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

You can always for example also search for text at the same time, working with MainWindow::filterNotes() is mandatory or you will immediately get bug reports. 😬

Copy link
Owner

Choose a reason for hiding this comment

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

Your new filters need to be part of the MainWindow::filterNotes() filtering process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can always for example also search for text at the same time, working with MainWindow::filterNotes() is mandatory or you will immediately get bug reports. grimacing

I just tried this.
Multi select + search in noteTreeWidget. Works, but if you delete the searched text, the selection isn't retained.

Copy link
Owner

Choose a reason for hiding this comment

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

Need more time to test that again, maybe tomorrow. I'm out the whole day today. 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright 😁

Copy link
Owner

Choose a reason for hiding this comment

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

Do you maybe have the 'recursive tagging' setting on? thinking

yes

https://imgur.com/a/7cr6b2a

Multiple sub-folders selected. Clicking on tags didn't change the note list.

Copy link
Owner

Choose a reason for hiding this comment

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

I am sorry, I tested one more time and didn't have the issue, I guess tray icon support shot me in the leg (an old version of the app may not have been closed). 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool. One more feature down. 🥇 😁

@coveralls
Copy link

coveralls commented Nov 27, 2019

Coverage Status

Coverage decreased (-0.08%) to 10.971% when pulling 485e5b1 on Waqar144:multisel into 4a54950 on pbek:develop.

@pbek pbek added the WIP PR that is still being worked on label Nov 29, 2019
@pbek pbek merged commit 0083adc into pbek:develop Dec 2, 2019
@Waqar144 Waqar144 deleted the multisel branch December 4, 2019 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP PR that is still being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants