Desktop: Focus is lost and notebook disappears (laurent22#10194) #10222

AliceHincu · 2024-03-28T00:42:39Z

The problem here arises from the fact that "All notes" has as selectedFolderId the value Folder, even though it should have the value SmartFilter. By switching those two lines, the type is correctly set. Before, "All notes" section was treated as a Folder, and for each note it wouldn't show from what notebook it came. After this modification, the UI is correct.

When the selected folder id is null, I set the default option to be the "All notes" folder.

Unfortunately I am unable to screen record to showcase this fix, because of some issues with my laptop, I am sorry.

github-actions · 2024-03-28T00:42:55Z

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

I have read the CLA Document and I hereby sign the CLA

2 out of 3 committers have signed the CLA.
✅ (AliceHincu)[https://github.com/AliceHincu]
✅ (personalizedrefrigerator)[https://github.com/personalizedrefrigerator]
❌ @alice Hincu
Alice Hincu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You can retrigger this bot by commenting recheck in this Pull Request}

AliceHincu · 2024-03-28T00:46:38Z

I have read the CLA Document and I hereby sign the CLA

HahaBill · 2024-03-28T09:52:54Z

Hi! Thank you for working on the issue. I made these changes, and yeah, I did not catch that. Calling the folderItem_click(ALL_NOTES_FILTER_ID); first is indeed better, in this case, to show what notes belong to what notebooks.

Can you check whether your changes affect Toggle own sort order for All notes? You can test it by right-clicking on the All Notes.

laurent22 · 2024-03-28T11:00:11Z

packages/app-desktop/gui/Sidebar/Sidebar.tsx

@@ -744,7 +744,7 @@ const SidebarComponent = (props: Props) => {


 	if (props.folders.length) {
-		const allNotesSelected = props.selectedFolderId === ALL_NOTES_FILTER_ID;
+		const allNotesSelected = props.selectedFolderId === null ? true : props.selectedFolderId === ALL_NOTES_FILTER_ID;


Does this mean that "All notes" can have as an ID either ALL_NOTES_FILTER_ID or null? And isn't it possible to ensure it always has ALL_NOTES_FILTER_ID as ID?

I think it's because of this existing check:

// Sidebar.tsx const folderItem_click = useCallback((folderId: string) => { props.dispatch({ type: 'FOLDER_SELECT', id: folderId ? folderId : null, }); }, [props.dispatch]);

But maybe AliceHincu found some issues or part of the codebase that I am unaware of.

I understand now. It's because when selecting a SmartFilter, the selectedSmartFilterId is updated, not
theselectedFolderId. And because now the code switches to the SmartFilter folder type later, then the selectedFolderId is null.

I suggest this: props.notesParentType === 'SmartFilter' && props.selectedSmartFilterId === ALL_NOTES_FILTER_ID and create a comment that SmartFilter folder types do not use selectedFolderId but selectedSmartFilterId

Yes that seems like a reasonable solution for now.

@AliceHincu, what is the status of this pull request? Do you think the changes suggested by HahaBill make sense, and if so could you apply them to the PR please?

Hi! I will implement these changes today and I will also check if there are any problems related to the change. I will be back with updates.

I updated the code. Also, I have noticed that we don't need the call to folderItem_click anymore, the code works fine without it... I also noticed that in tests, when wanting to test the All Notes section, dispatching the action SMART_FILTER_SELECT is enough. I do not know if I am missing something, since I can't see how it can be useful for us anymore. I switched between having the call and not having the call and I don't see any differences.

Basically, I added folderItem_click for the feature where you can have custom ordering for notes in All notes. However, I think that is the out of the scope of this PR. I suggest that I would create another PR for that, let's see what laurent22 has to say.

Anyway, in my opinion the code looks good for now :)

laurent22 · 2024-03-28T12:03:38Z

By the way does this pull request need to be labelled for GSoC?

HahaBill · 2024-03-28T12:32:36Z

Update: Your changes will not work for the Toggle own sort order. However, because I understand much better the Joplin codebase than few weeks ago, we can make the solution smarter and not change the folder type twice by doing this:

// In togglePerFolderSortOrder.ts
// Added another parameter called: folderName

execute: async (_context: CommandContext, folderId?: string, own?: boolean, folderName?: string) => {
	    PerFolderSortOrderService.set(folderId, own, folderName);
},

// Siderbar.tsx
const onAllNotesClick_ = useCallback(() => {
      props.dispatch({
	      type: 'SMART_FILTER_SELECT',
	      id: ALL_NOTES_FILTER_ID,
      });
      PerFolderSortOrderService.selectNonFolder('All Notes');
}, [props.dispatch]);

...menuUtils.commandToStatefulMenuItem('togglePerFolderSortOrder', ALL_NOTES_FILTER_ID, undefined, 'All Notes'),

// PerFolderSortOrderService.ts
// We will dynamically change the folderName

public static selectNonFolder(folderName: string) {
	this.folderName = folderName;
}

public static set(folderId?: string, own?: boolean, folderName?: string) {
	let targetId = folderId;
	if (folderName !== undefined) {
		this.folderName = folderName;
	} else {
		this.folderName = null;
	}
        .
        .
}

private static getSelectedFolderId(): string {
        if (this.folderState.notesParentType === 'Folder') {
	        return this.folderState.selectedFolderId;
        } else if (this.folderState.notesParentType === 'SmartFilter') {
	        if (this.folderName === 'All Notes') {
		        return ALL_NOTES_FILTER_ID;
	        }
        } else {
	        return '';
        }
}

This works and solves the bug :)

Also, this solves the problem of other folders being SmartFilter as well as we discussed in our PR, if you remember laurent22. The solution will keep things decoupled.

HahaBill · 2024-03-28T13:53:28Z

My fixes might be out of the scope of the PR.

I suggest that we use AliceHincu's implementation to fix the bug as fast as possible since this issue is a high priority and the deadline for the GSoC application is approaching soon. When this PR is merged, I will fix the Toggle own sort order in another PR.

Aitchessbee · 2024-03-28T15:46:52Z

My fixes might be out of the scope of the PR.

I suggest that we use AliceHincu's implementation to fix the bug as fast as possible since this issue is a high priority and the deadline for the GSoC application is approaching soon. When this PR is merged, I will fix the Toggle own sort order in another PR.

Any comments on the approach I tried in #10220? To me, the approach in this PR looks better... (Edit- by this, I meant Alice's PR)

HahaBill · 2024-03-28T17:42:00Z

My fixes might be out of the scope of the PR.
I suggest that we use AliceHincu's implementation to fix the bug as fast as possible since this issue is a high priority and the deadline for the GSoC application is approaching soon. When this PR is merged, I will fix the Toggle own sort order in another PR.

Any comments on the approach I tried in #10220? To me, the approach in this PR looks better...

Thanks for looking at the issue, too! I had a look and tried to test it, but unfortunately, your changes do not fix the current bug for this issue.

AliceHincu · 2024-03-29T08:20:40Z

Hi! Sorry for not replying earlier. Yes, this should be labeled for gsoc.

laurent22 · 2024-03-29T12:03:24Z

@AliceHincu, ok that's done. Also please remember that the deadline to submit your application is 2 April so if you haven't done so already consider preparing it in parallel. You can also send us a PM so that we can review your project proposal before you submit it as described here: https://discourse.joplinapp.org/t/the-gsoc-application-deadline-is-on-april-2/37107/2

laurent22 · 2024-04-10T14:25:01Z

@AliceHincu, it looks good now but there's a conflict. Could you please fix it and then we can merge?

personalizedrefrigerator · 2024-04-15T16:59:10Z

It looks like 89acde9 has incorrect authorship information (uses an email not associated with the @AliceHincu GitHub account). This seems to be causing the CLA Assistant check to fail.

This StackOverflow post may be helpful in changing the author information for 89acde9.

AliceHincu · 2024-04-16T06:31:23Z

Hi, sorry for not responding earlier, I was sick for a few days. I will take a look at the article today to fix the issue

laurent22 · 2024-04-25T13:10:50Z

Thanks for looking into this but this issue has now been fixed in #10370

Dricc123 · 2024-04-28T19:51:50Z

Not sure where I shall post this because there are many merged/duplicate issues but I still have something wrong going on related to that. most of it has been fixed, but when I'm in the 'all notes' folder and search for a word, I then go in one of the results of my search and modify the note, and there the note disappears and it jumps to the next note

HahaBill · 2024-04-28T19:59:55Z

Not sure where I shall post this because there are many merged/duplicate issues but I still have something wrong going on related to that. most of it has been fixed, but when I'm in the 'all notes' folder and search for a word, I then go in one of the results of my search and modify the note, and there the note disappears and it jumps to the next note

This particular issue can be found here: #10236

Desktop: Focus is lost and notebook disappears (laurent22#10194)

89acde9

laurent22 reviewed Mar 28, 2024

View reviewed changes

laurent22 added the gsoc-2024 label Mar 29, 2024

HahaBill mentioned this pull request Mar 29, 2024

Focus is lost and notebook disappears #10194

Closed

Merge branch 'laurent22:dev' into fix-issue-10194

f21fbc9

github-actions bot added a commit that referenced this pull request Apr 4, 2024

@AliceHincu has signed the CLA in #10222

660ebcf

AliceHincu and others added 3 commits April 4, 2024 23:18

Merge branch 'laurent22:dev' into fix-issue-10194

5c7a02c

Desktop: Focus is lost and notebook disappears

66c2a0b

Desktop: Focus is lost and notebook disappears

a5f6688

personalizedrefrigerator linked an issue Apr 15, 2024 that may be closed by this pull request

Focus is lost and notebook disappears #10194

Closed

Merge branch 'dev' into fix-issue-10194

d5bf06e

personalizedrefrigerator mentioned this pull request Apr 16, 2024

Note display / selection changes during text input #10318

Closed

personalizedrefrigerator mentioned this pull request Apr 24, 2024

Desktop: Fixes #10194: Fix note disappears while editing #10370

Merged

laurent22 closed this Apr 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Desktop: Focus is lost and notebook disappears (laurent22#10194) #10222

Desktop: Focus is lost and notebook disappears (laurent22#10194) #10222

AliceHincu commented Mar 28, 2024

github-actions bot commented Mar 28, 2024 •

edited

Loading

AliceHincu commented Mar 28, 2024

HahaBill commented Mar 28, 2024

laurent22 Mar 28, 2024

HahaBill Mar 28, 2024 •

edited

Loading

HahaBill Mar 28, 2024 •

edited

Loading

laurent22 Apr 1, 2024

laurent22 Apr 3, 2024

AliceHincu Apr 4, 2024

AliceHincu Apr 4, 2024

HahaBill Apr 4, 2024

laurent22 commented Mar 28, 2024

HahaBill commented Mar 28, 2024 •

edited

Loading

HahaBill commented Mar 28, 2024 •

edited

Loading

Aitchessbee commented Mar 28, 2024 •

edited

Loading

HahaBill commented Mar 28, 2024

AliceHincu commented Mar 29, 2024

laurent22 commented Mar 29, 2024

laurent22 commented Apr 10, 2024

personalizedrefrigerator commented Apr 15, 2024

AliceHincu commented Apr 16, 2024

laurent22 commented Apr 25, 2024

Dricc123 commented Apr 28, 2024 •

edited

Loading

HahaBill commented Apr 28, 2024

Desktop: Focus is lost and notebook disappears (laurent22#10194) #10222

Desktop: Focus is lost and notebook disappears (laurent22#10194) #10222

Conversation

AliceHincu commented Mar 28, 2024

github-actions bot commented Mar 28, 2024 • edited Loading

AliceHincu commented Mar 28, 2024

HahaBill commented Mar 28, 2024

laurent22 Mar 28, 2024

Choose a reason for hiding this comment

HahaBill Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

HahaBill Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

laurent22 Apr 1, 2024

Choose a reason for hiding this comment

laurent22 Apr 3, 2024

Choose a reason for hiding this comment

AliceHincu Apr 4, 2024

Choose a reason for hiding this comment

AliceHincu Apr 4, 2024

Choose a reason for hiding this comment

HahaBill Apr 4, 2024

Choose a reason for hiding this comment

laurent22 commented Mar 28, 2024

HahaBill commented Mar 28, 2024 • edited Loading

HahaBill commented Mar 28, 2024 • edited Loading

Aitchessbee commented Mar 28, 2024 • edited Loading

HahaBill commented Mar 28, 2024

AliceHincu commented Mar 29, 2024

laurent22 commented Mar 29, 2024

laurent22 commented Apr 10, 2024

personalizedrefrigerator commented Apr 15, 2024

AliceHincu commented Apr 16, 2024

laurent22 commented Apr 25, 2024

Dricc123 commented Apr 28, 2024 • edited Loading

HahaBill commented Apr 28, 2024

github-actions bot commented Mar 28, 2024 •

edited

Loading

HahaBill Mar 28, 2024 •

edited

Loading

HahaBill Mar 28, 2024 •

edited

Loading

HahaBill commented Mar 28, 2024 •

edited

Loading

HahaBill commented Mar 28, 2024 •

edited

Loading

Aitchessbee commented Mar 28, 2024 •

edited

Loading

Dricc123 commented Apr 28, 2024 •

edited

Loading