Skip to content

Conversation

@Pudge601
Copy link
Contributor

This ensures that the newly created note is visible and selected in the note list.

Fixes #2241

@ZeroX-DG ZeroX-DG added the awaiting review ❇️ Pull request is awaiting a review. label Sep 15, 2018
@ZeroX-DG
Copy link
Member

You code will not focus the correct note if there is one note that matched in the search box.
boostnote

@ZeroX-DG ZeroX-DG added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Sep 15, 2018
The note which we are jumping to may not be available in the note list for a number of reasons (e.g. if there is an active search, or if another storage folder is selected, or if the note list is showing starred notes).

This affects both when we are creating a new note (which may not match the current search criteria), and when jumping to a note via a link in another note (and the linked note may not be available for any of the above reasons).

2241
@Pudge601
Copy link
Contributor Author

Hmm... that's true.

The more I'm looking into this issue, the more confused I'm getting. I'm starting to think that the root issue is that the code for jumping to a note by hash (using the list:jump event) doesn't work if the note is not visible in the note list.

That event gets fired when a new note is created (hence this issue), and also when clicking a link to a note from another note (browser/components/MarkdownPreview.js:826). I've tested and found the same issue when clicking a note link; if there is a search active which the linked note does not match, then it will not jump to the linked note.

bootnote-jump-search

However, just to add to my confusion, it is possible to jump to a linked note which isn't in the current note list when looking at Starred notes, or notes within a particular Storage folder - in this case, the note is shown in the preview, but doesn't appear in the notes list on the left! I have now idea how this behaviour works, but I'm tempted to say that it is a bug in itself (or at least unintended behaviour), since it leads to a very weird scenario where the note which is in the preview is not the focused note in the note list, and in fact doesn't even appear in the note list.

bootnote-jump-starred
bootnote-jump-storage

This has led me to thinking that perhaps it should be switching to the /home route whenever we jump to a note by its hash. This provides consistent behaviour for all of these scenarios;

  • Creating a note with search active
  • Jumping to a linked note with search active
  • Jumping to a linked note while looking at Starred notes
  • Jumping to a linked note while looking at a Storage folder
@Pudge601
Copy link
Contributor Author

I've changed my fix in this PR to switch to the '/home' route when jumping to a note by hash. This should solve all of the issues described in my last comment in a consistent manner.

@ZeroX-DG
Copy link
Member

ZeroX-DG commented Oct 9, 2018

Is it because of me or the old bug is still there?

@ZeroX-DG
Copy link
Member

ZeroX-DG commented Nov 28, 2018

@Pudge601 What's the status of this?
EDIT Sorry for bothering you. Somehow it works fine now.

@ZeroX-DG ZeroX-DG added approved 👍 Pull request has been approved by sufficient reviewers. and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Nov 28, 2018
Copy link
Member

@ZeroX-DG ZeroX-DG left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@ZeroX-DG
Copy link
Member

Also fixed this: #2647

@Rokt33r Rokt33r added next release (v0.11.12) and removed approved 👍 Pull request has been approved by sufficient reviewers. labels Nov 28, 2018
@Rokt33r Rokt33r merged commit 830ade9 into BoostIO:master Nov 28, 2018
@Pudge601
Copy link
Contributor Author

Sorry, I completely missed your comment back in October about it not working :S not sure why it wouldn't have worked when you tested it, but glad it's all sorted now 👍

@Pudge601 Pudge601 deleted the bug/2241 branch November 28, 2018 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants