Desktop: Assign note to Notebook or Sub_Notebook functionality added #2566

coderrsid · 2020-02-23T22:16:59Z

Fixed Issue #2296

This pull request add the functionality of assigning note to another notebook.

WORKING

coderrsid · 2020-02-23T22:18:03Z

Special thanks to @tessus for helping me out for every query. I seek to participate in Google Summer of Code and work with Joplin.

laurent22 · 2020-02-23T23:23:53Z

The animated gif doesn't seem to work. How does your code handle sub notebooks?

tessus · 2020-02-24T00:23:42Z

I'm not sure but the animated gif only shows how a note is created. This PR is for moving one or more notes to an existing folder.

coderrsid · 2020-02-24T06:00:46Z

The animated gif doesn't seem to work. How does your code handle sub notebooks?

Hey @laurent22, Thanks for asking. I just updated the GIF, actually it was not the whole GIF earlier. Now you can see we can assign note to any notebook or sub notebook.

coderrsid · 2020-02-24T06:01:51Z

I'm not sure but the animated gif only shows how a note is created. This PR is for moving one or more notes to an existing folder.

Hey @tessus, actually it was not the whole GIF earlier. Now you can see we can assign note to any notebook or sub notebook.

tessus · 2020-02-24T06:25:55Z

Thanks for the new gif.

I have 2 questions:

how is the search implemented? e.g. if you start with a, does it show all the folders that start with a or that contain a?
the folder structure is flat. Is there any chance that sub folders are indented relative to their parents?

tessus · 2020-02-24T06:30:48Z

There must be a code already available that does the folder list with indentions. The web clipper allows you to choose a folder from a list which has the proper structure. It's also available on the mobile client, when you move a note to a different folder. These 2 just don't allow you to search.

But maybe the indention is just a stupid idea, but I like it when I have a structure.

coderrsid · 2020-02-24T06:39:19Z

There must be a code already available that does the folder list with indentions. The web clipper allows you to choose a folder from a list which has the proper structure. It's also available on the mobile client, when you move a note to a different folder. These 2 just don't allow you to search.

But maybe the indention is just a stupid idea, but I like it when I have a structure.

Search implementation is already done @tessus . If you want the subfolders indented too? I can work on that.

tessus · 2020-02-24T06:44:56Z

Ok, I can see it now. It's just a bit confusing, because in the beginning, when there's just an a in the search field, it shows all folders. This should not be the case.
But I was asking how the search algorithm works. But I think I figured it from the gif. It lists all folders that contain the search string (and not start with it).

If you want the subfolders indented too?

Let's wait for Laurent's input.

miciasto · 2020-02-24T07:40:58Z

Hi @coderrsid the text in the video is small. Is it possible to zoom into the relevant part of screen to show clearly what is happening?

coderrsid · 2020-02-24T07:51:30Z

@tessus. It's the defaultValue.. which i have set to the folder[0] (first folder in the list), I can change it to empty value too

tessus · 2020-02-24T08:11:09Z

Yep, I think it should be empty by default, in which case it should show all folders (with indention). As soon as you start typing only the results are shown. And they don't have to be indented.
What do you think? Does this make sense?

tessus · 2020-02-24T08:20:12Z

When I come to think of it, we have missed something: Folders can have the same name.

Take the following structure:

A
   B
   C
D
   B
   E
F
   G
   H
   I
      B

When you search for B, you will get three B in the result set:

B
B
B

Even with indention, we can't solve this problem.

laurent22 · 2020-02-24T09:28:00Z

If you want the subfolders indented too?

Yes it should be indented. Please have a look at how it's done in the clipper for example.

coderrsid · 2020-02-24T13:36:56Z

When I come to think of it, we have missed something: Folders can have the same name.

Take the following structure:
A
   B
   C
D
   B
   E
F
   G
   H
   I
      B
When you search for B, you will get three B in the result set:
B
B
B
Even with indention, we can't solve this problem.

I believe that too.. @tessus .

coderrsid · 2020-02-24T13:38:18Z

If you want the subfolders indented too?

Yes it should be indented. Please have a look at how it's done in the clipper for example.

Yeah i'll have a look. Can you direct me to the clipper source indent files ?

coderrsid · 2020-02-24T14:49:24Z

@laurent22 . I tried changing the code according to the clipper but it seems that the function nonBreakingSpecify() is not saving spaces correctly or they are truncated by some default value.

But when i tried to console.log, the updated title doesn't have the correct spaces as you can compare the code(above) and the console in the (bottom of) screenshot below.

tessus · 2020-02-24T23:29:11Z

I believe that too.. @tessus .

Yes, but there are a few ideas that will solve the problem.

coderrsid · 2020-02-25T03:21:36Z

I believe that too.. @tessus .

Yes, but there are a few ideas that will solve the problem.

Yeah @tessus. One way it can be solved is by indenting folders like in the clipper @laurent22 suggested by using depth property. We can change the title of the folder in the display by which it shows the name like the image below according to the hierarchy

tessus · 2020-02-25T03:32:12Z

No, I think you do not understand. Look at my example above. When you search for B you will get 3 results:

B
B
B

Even with indention it would look like this:

   B
   B
      B

So, how exactly does this help?

There are 2 issues here:

The part where nothing is in the search field. This is the part where indention makes sense.
When you enter a search term and the search result set contains duplicates.

I'm talking about the 2nd item. Indention will do nothing. So you will have to find a solution.
E.g. in case of duplicate enttries in the result set, also show the parents.

B (A)
B (D)
B (F <separator> I)

Do you understand what I'm trying to say?

coderrsid · 2020-02-25T03:39:31Z

No, I think you do not understand. Look at my example above. When you search for B you will get 3 results:
B
B
B
Even with indention it would look like this:
   B
   B
      B
So, how exactly does this help?

There are 2 issues here:

The part where nothing is in the search field. This is the part where indention makes sense.

When you enter a search term and the search result set contains duplicates.

I'm talking about the 2nd item. Indention will do nothing. So you will have to find a solution.
E.g. in case of duplicate enttries in the result set, also show the parents.
B (A)
B (D)
B (F <separator> I)
Do you understand what I'm trying to say?

Yeah yeah i do understand what you're saying instead of parent's name after child.
What we can do instead is that if we search B:
(d) = disabled or greyed out

 B
 AB(d)
         B
 AC(d)
        A(d)
               B

I'd prefer this though? What do you suggest?

tessus · 2020-02-25T03:46:50Z

Hmm, not a bad idea, but I think you meant:

A (disabled)
   B
D (disabled)
   B
F (disabled)
   I (disabled)
      B

Yes, this is also a solution. Mine was just an example. And since we most likely won't have too many duplicates anyway, the list should not be too long.

Theoretically you could even use this algorithm for non duplicates. In that case it would be awesome, if we could only jump between the active ones, so that one doesn't have to hit 6x the down key to actually reach the 3rd result.

coderrsid · 2020-02-25T04:21:25Z

Yeah sure we can. That's why i suggested this method. Disabling items will lead to an easy search and will show the parent of sub-notebook too and as you said for moving down in the list too. Can we confirm this from @laurent22 too?

tessus · 2020-02-25T06:04:40Z

Yep, Laurent should definitely chime in.

miciasto · 2020-02-25T06:46:01Z

if we could only jump between the active ones, so that one doesn't have to hit 6x the down key to actually reach the 3rd result.

Yes, but using the mouse it won't be so much fun. I think the concise option you first suggested is preferable.

tessus · 2020-03-13T18:37:50Z

Well, [string]: any could also include HTML characters like  . Your previous comment does not say anything about the string being processed literally.

laurent22 · 2020-03-16T17:30:13Z

@coderrsid, you need to dig deeper here. The library doesn't support what you need? Then perhaps create a pull request on their repo to support it, or create a patch, or try a different component, etc. We can make suggestions on how to fix it, but you need to make some too, and try alternatives by yourself.

It needs to work with a notebook hierarchy, or if it's not possible we close the pull request. So please confirm if you're still interested looking into it.

coderrsid · 2020-03-16T18:40:55Z

@laurent22 Thanks for replying. Yeah sure I can dig deeper. It's just that i want us to stay on the same page. It shouldn't be like i spend some hours onto something which is not permissible and PackElend explicitly mentioned to communicate. That's why i am asking you guys for every step. Sorry if i am making it too much of a trouble. 😄

PS: I am working on this PR

coderrsid · 2020-03-18T14:10:11Z

I tried almost every method but unfortunately i guess the react-select doesn't support adding spaces to their options of <Select /> component. So i've created an issue in their repository. Link is below. You guys can also comment there, this would be great for us and their repo too. Thanks.
JedWatson/react-select#3975

PackElend · 2020-03-18T16:06:59Z

So i've created an issue in their repository. Link is below.

😨

coderrsid · 2020-03-21T08:21:20Z

@PackElend soo what else can we do?

PackElend · 2020-03-21T08:46:49Z

@PackElend soo what else can we do?

is there an alternative library?

coderrsid · 2020-03-21T08:52:56Z

is there an alternative library?

yes, if we do try to find but instead of adding another library or package, can't. we render a JSX based select instead of using it from a library for just this case Assign Notebook?

PackElend · 2020-03-21T13:05:50Z

this not my call, I cannot help here.
Let's wait for the others
@bedwardly-down I drag you in here too, you might be interested

coderrsid · 2020-03-21T13:10:35Z

Ohh sure @PackElend, i am happy for your participation.

bedwardly-down · 2020-03-21T15:06:02Z

I am going to be personally frank, I’ve been following this PR for a good while now and am exhausted of it. @coderrsid, if you can endure and follow through here, i will be impressed and amazed and would pretty much accept anything you want to do with Joplin for your project but also see that you failing would be a massive time sink for everyone involved here. I hope you get this completed and prove my fears and concerns wrong.

On the library question, I’m more than willing to help you figure that out but would also need to research it a bit because I don’t fully understand what all this PR is attempting to do just yet. I also know that many react native libraries seem to be getting abandoned as Facebook, its developer and maintainer, break compatibility through updates and whatnot.

coderrsid · 2020-03-21T15:48:39Z

I hope you get this completed and prove my fears and concerns wrong.

Thanks @bedwardly-down . I will try my best for that.

I don’t fully understand what all this PR is attempting to do just yet.

We are just adding a functionality of assigning single or multiple notes to a notebook directly without the use of mouse-drag but using a dropdown like shown in the description of PR.
As notebooks can contain sub-notebooks, we should use a tree hierarchy structure for listing notebook names in the dropdown like :

instead of

but the package react-select (used for rendering dropdown) doesn't support this and now we can try two different methods :

We can try a different package
We can render a React based dropdown without any package as used in CliClient above in the first GIF
We can wait for Whitespaces gets trimmed in select options when making tree hierarchy JedWatson/react-select#3975 to resolve

laurent22 · 2020-03-24T17:42:44Z

@coderrsid, should we close this pull request as it's been nearly a month, many comments and people involved and it's still not done. Maybe it's too complicated to implement so it's perhaps time to cut our losses and close?

coderrsid · 2020-03-24T17:46:27Z

@laurent22 , please do not close this pull request, i am trying different libraries like react-dropdown-tree-select for tree type hierarchy. But will you accept adding another package to the application because the existing packages will not be able to pull up a tree hierarchy in the prompt-options dropdown?

PackElend · 2020-03-24T20:35:31Z

@laurent22 , please do not close this pull request, i am trying different libraries like react-dropdown-tree-select for tree type hierarchy.

Time is rare these days so you can continue working on it but Laurent won't react so please do not mention him anymore in this PR.
You can keep trying but you are on your own. You can ask in the forum for verification support as we need him and others elsewhere at the moment.

Your efforts and successful PRs have been recognized no worries :)
May you can help out others too here and in the forum.

coderrsid · 2020-03-24T21:12:37Z

Yeah i know PackElend, sure i won't. I'll just try to implement it myself and i'll come with a solution very very soon. But please don't close this pull request. I've been working on this too along with everyone.
Sure i'll help as much as i can. Thank you so much.

Jackymancs4 · 2020-03-25T15:25:45Z

Hello,
I have been following this PR for some times now, mainly out of scientific interest. I think I found a viable solution that does not require a new package to be added.

It leverages the styling API (https://react-select.com/styles) of react-select to adjust the left padding of a single option to a fixed amount.

The commit with the changes can be found here: Jackymancs4@063a164 in https://github.com/Jackymancs4/joplin/tree/develop/assign-notebook

The code is just a mockup. To be functional this line https://github.com/Jackymancs4/joplin/blob/063a1649a0d2459ff62b746cb698163d52b52320/ElectronClient/gui/MainScreen.jsx#L215 needs to implement a proper logic to return the depth level of any notebook.

I hope this helps.

coderrsid · 2020-03-26T09:46:55Z

Hey @Jackymancs4 , thank you so much for figuring that out. You're right, it's about the styles in package. I tried implying the code by passing folder's indentDepth values in MainScreen.jsx as :

else if (command.name === 'moveToFolder') {
			const startFolders = [];
			
            const addOptions = (folders, depth) => {
                for(let i = 0; i < folders.length; i++) {
                    const folder = folders[i];
                    startFolders.push({ key: folder.id, value: folder.id, label: folder.title, indentDepth: depth });
                    if(folder.children) addOptions(folder.children, depth + 1);
                }
            }

            addOptions(this.props.folders, 0);

			this.setState({
				promptOptions: {
					label: _('Move to notebook:'),
					inputType: 'dropdown',
					value: '',
					autocomplete: startFolders,
					onClose: async answer => {
						if (answer != null) {
							for (let i = 0; i < command.noteIds.length; i++) {
								await Note.moveToFolder(command.noteIds[i], answer.value);
							}
						}
						this.setState({ promptOptions: null });
					},
				},
			});

and the changes in PromptDialog.jsx are as :

option: (provided, state) => {
				console.log(state.data);
				Object.assign(provided, {
					color: theme.color,
					fontFamily: theme.fontFamily,
					paddingLeft: `${10 + state.data.indentDepth * indentDepthWidth}px`,
				})},

But still when i console.log the state.data in the PromptDialog.jsx, it's passing 0 as the value for all folders as shown in the ss below.

I tried your way laurent as it is implied in Clipper. Can you tell why this is happening?

Jackymancs4 · 2020-03-26T12:35:17Z

Well, this happen because this.props.folders does not contains children for any note (most likely it has been generated by Folder.all()).

In my last commit here 979e8ec you can see a viable solution using await Folder.allAsTree().

There I also added a mechanism to limit notebook depth, for good measure.
The result is fairly pleasant.

coderrsid · 2020-03-26T13:15:57Z

Well, thanks for the viable solution and i just tried testing the changes, it renders them perfectly i'll just push the changes with some changes in styles. Thanks @Jackymancs4 for helping out big here!

laurent22 · 2020-04-14T09:38:38Z

@coderrsid, there's some issue with the sorting order: #3052 Do you know what might be the problem?

coderrsid · 2020-04-14T09:48:57Z

Sure laurent, i'll just look into it.

Assign Notebook functionality added

dc42ac9

tessus changed the title ~~(Desktop) Assign note to Notebook functionality added~~ Desktop: Assign note to Notebook functionality added Feb 24, 2020

tessus added the desktop All desktop platforms label Feb 24, 2020

tessus linked an issue Feb 24, 2020 that may be closed by this pull request

Right-click to assign a note to a notebook #2296

Closed

coderrsid changed the title ~~Desktop: Assign note to Notebook functionality added~~ Desktop: Assign note to Notebook or Sub_Notebook functionality added Feb 24, 2020

PackElend added the upstream There's a problem with upstream code. label Mar 18, 2020

Merge remote-tracking branch 'upstream/master' into Assign_Notebook

80c5bc5

coderrsid and others added 2 commits March 26, 2020 18:48

Render notebooks in tree hierarchy

e7bf8a6

Update PromptDialog.jsx

94e4cd1

laurent22 merged commit 1e9a003 into laurent22:master Apr 4, 2020

coderrsid deleted the Assign_Notebook branch April 4, 2020 17:16

laurent22 removed the upstream There's a problem with upstream code. label Apr 14, 2020

bladey mentioned this pull request Jun 19, 2020

Whitespaces gets trimmed in select options when making tree hierarchy JedWatson/react-select#3975

Closed

Desktop: Assign note to Notebook or Sub_Notebook functionality added #2566

Desktop: Assign note to Notebook or Sub_Notebook functionality added #2566

Conversation

coderrsid commented Feb 23, 2020 • edited Loading

WORKING

coderrsid commented Feb 23, 2020

laurent22 commented Feb 23, 2020

tessus commented Feb 24, 2020

coderrsid commented Feb 24, 2020

coderrsid commented Feb 24, 2020 • edited Loading

tessus commented Feb 24, 2020 • edited Loading

tessus commented Feb 24, 2020

coderrsid commented Feb 24, 2020 • edited Loading

tessus commented Feb 24, 2020

miciasto commented Feb 24, 2020

coderrsid commented Feb 24, 2020

tessus commented Feb 24, 2020

tessus commented Feb 24, 2020

laurent22 commented Feb 24, 2020

coderrsid commented Feb 24, 2020

coderrsid commented Feb 24, 2020

coderrsid commented Feb 24, 2020 • edited Loading

tessus commented Feb 24, 2020

coderrsid commented Feb 25, 2020

tessus commented Feb 25, 2020

coderrsid commented Feb 25, 2020 • edited Loading

tessus commented Feb 25, 2020

coderrsid commented Feb 25, 2020

tessus commented Feb 25, 2020

miciasto commented Feb 25, 2020

tessus commented Mar 13, 2020

laurent22 commented Mar 16, 2020

coderrsid commented Mar 16, 2020

coderrsid commented Mar 18, 2020

PackElend commented Mar 18, 2020

coderrsid commented Mar 21, 2020

PackElend commented Mar 21, 2020

coderrsid commented Mar 21, 2020 • edited Loading

PackElend commented Mar 21, 2020

coderrsid commented Mar 21, 2020

bedwardly-down commented Mar 21, 2020

coderrsid commented Mar 21, 2020

instead of

laurent22 commented Mar 24, 2020

coderrsid commented Mar 24, 2020

PackElend commented Mar 24, 2020

coderrsid commented Mar 24, 2020

Jackymancs4 commented Mar 25, 2020

coderrsid commented Mar 26, 2020

Jackymancs4 commented Mar 26, 2020

coderrsid commented Mar 26, 2020 • edited Loading

laurent22 commented Apr 14, 2020

coderrsid commented Apr 14, 2020

coderrsid commented Feb 23, 2020 •

edited

Loading

coderrsid commented Feb 24, 2020 •

edited

Loading

tessus commented Feb 24, 2020 •

edited

Loading

coderrsid commented Feb 24, 2020 •

edited

Loading

coderrsid commented Feb 24, 2020 •

edited

Loading

coderrsid commented Feb 25, 2020 •

edited

Loading

coderrsid commented Mar 21, 2020 •

edited

Loading

coderrsid commented Mar 26, 2020 •

edited

Loading