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

"move to notebook" list not sorted in alphabetic order #3052

Closed
foxmask opened this issue Apr 14, 2020 · 21 comments · Fixed by #3084
Closed

"move to notebook" list not sorted in alphabetic order #3052

foxmask opened this issue Apr 14, 2020 · 21 comments · Fixed by #3084
Labels
bug It's a bug high High priority issues

Comments

@foxmask
Copy link
Contributor

foxmask commented Apr 14, 2020

Environment

Joplin version: 1.0.200
Platform: linux
OS specifics: debian

Steps to reproduce

  1. when moving note to another notebook the popup list is not sorted in alpha order
@foxmask foxmask added the bug It's a bug label Apr 14, 2020
@foxmask foxmask changed the title "move to notebook" list not sorted in alphabetique order "move to notebook" list not sorted in alphabetic order Apr 14, 2020
@laurent22 laurent22 added the high High priority issues label Apr 14, 2020
@coderrsid
Copy link
Contributor

Hey @foxmask, thanks for reporting. I just checked again for the error you specified, but i don't see any. You can see in the below GIF it is rendering properly according to the notebooks listed in sidebar.

Screen-Recording-2020-04-14-at-3

Can you provide any screenshot or GIF for the problem you're facing? 😄

@foxmask
Copy link
Contributor Author

foxmask commented Apr 14, 2020

@coderrsid you can see a screencast I made here #3053 when I move note ; you can see "Tech" before "Projet" before "Maison" it looks like sorted in the other side.

@coderrsid
Copy link
Contributor

Thanks for telling but that link is expired and can i ask why we have two different issues for a similar problem?
image

@foxmask
Copy link
Contributor Author

foxmask commented Apr 14, 2020

because it's 2 issues ...

@coderrsid
Copy link
Contributor

image

Although i see there is a sorting issue in here. I'll just fix this ASAP.

@foxmask
Copy link
Contributor Author

foxmask commented Apr 14, 2020

@coderrsid
Copy link
Contributor

Great. Now i can see the issue. Thanks for reporting. 😄

@tessus
Copy link
Collaborator

tessus commented Apr 14, 2020

@coderrsid btw, the cursor looks really weird in the dialog:

mtnb

Maybe, because it doesn't blink. It only starts blinking when you pick a folder.

@coderrsid
Copy link
Contributor

Thanks @tessus . I'll look into this too.

@coderrsid
Copy link
Contributor

@foxmask Can you add the GIF here too please. Unfortunately, the last one is removed again. 😄

@coderrsid
Copy link
Contributor

And one more thing i want to ask is that the order of notebooks shown in your sidebar is perfectly right, the way it should be?

@foxmask
Copy link
Contributor Author

foxmask commented Apr 16, 2020

yes the order in the left sidebar is the right one.

here is the gif
joplin-move-bug

@coderrsid
Copy link
Contributor

Thanks! I'll see what i can do.

@coderrsid
Copy link
Contributor

@laurent22 Will it be right if i change the way how Folder.allAsTree( ) returns the folders tree in a sorted way instead of just rendering parent and children together but also sort them or instead i should just make them sort for the moveToNotebook . I believe it will be helpful in future use cases.

@coderrsid
Copy link
Contributor

@tessus In my case, the cursor is working as expected. Although if it is an issue, it should be in all the dialog boxes using inputType: 'dropdown' . Can you check the cursor in the dialog box of Create note from Template or Create to-do from template or Insert Template
Screen-Recording-2020-04-18-at-4 (2)

@tessus
Copy link
Collaborator

tessus commented Apr 18, 2020

Will it be right if i change the way how Folder.allAsTree( ) returns the folders tree in a sorted way

How is this done by the web clipper? I believe the function returns a sorted result set. Either way, something like a boolean argument with a default is a good option (sort = true). On the other hand, why would someone ever want a not sorted result?
But maybe people would like the same order they set in Sort notebooks by. This is certainly an architectural question and this is something only Laurent can answer.
If it were me, I'd create a function that takes the sort order as argument, which is called with the order that is set in the menu. IMO that would be the most logical and consistent approach.

Can you check the cursor in the dialog box of Create note from Template or Create to-do from template or Insert Template

In those cases the cursor blinks right away. Not so with the Move to notebook dialog.

@coderrsid
Copy link
Contributor

How is this done by the web clipper? I believe the function returns a sorted result set. Either way, something like a boolean argument with a default is a good option (sort = true). On the other hand, why would someone ever want a not sorted result?

That's why i asked laurent if i can change it to always return a sorted array by title. 😄

But maybe people would like the same order they set in Sort notebooks by. This is certainly an architectural question and this is something only Laurent can answer.

Yeah that would be something he can answer for sure.

If it were me, I'd create a function that takes the sort order as argument, which is called with the order that is set in the menu. IMO that would be the most logical and consistent approach.

that's a great idea, we can add a function which returns the order of the notebooks in the same option set in Sort notebooks by

In those cases the cursor blinks right away. Not so with the Move to notebook dialog.

Although it shouldn't have any problem as the dialog is a part of a simple dropdown which is similarly used in the other components i ask you to test in .. 😅 Still i'll just look at it again but do tell me how to reproduce the issue. Than k you so much for suggesting tessus. 😄

@coderrsid
Copy link
Contributor

Will it be right if i change the way how Folder.allAsTree( ) returns the folders tree in a sorted way instead of just rendering parent and children together but also sort them or instead i should just make them sort for the moveToNotebook . I believe it will be helpful in future use cases.

@PackElend @bedwardly-down
what do you think about this?

@bedwardly-down
Copy link
Contributor

This is honestly less us and more the dev team. Tessus and Laurent didn’t respond to that post directly, so I’d take that as them probably being indifferent to it. With everything going on, it might be better to take a step back here also and just patiently wait for more feedback or ask on the forums.

@coderrsid
Copy link
Contributor

Sure. As you say sir.

@tessus
Copy link
Collaborator

tessus commented Apr 18, 2020

@coderrsid

but do tell me how to reproduce the issue.

Not really much to it. right-click a note and click Move to notebook. The cursor does not blink.

laurent22 pushed a commit that referenced this issue May 9, 2020
…dialog (#3084)

* Added Folder::sortFolderTree

* Added unit tests
@lock lock bot locked and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug It's a bug high High priority issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants