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

Mobile: Fixes: #10677: Following a link to a previously open note wouldn't work #10750

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

pedr
Copy link
Collaborator

@pedr pedr commented Jul 15, 2024

Fixes #10677

Description

A common source of problems in mobile seems to be that the NoteScreenComponent needs a lot of "hacks" to be able to change from one note content to another.

In this issue I'm addressing the latest bug reported that has a similar source of problem.

The original problem was reported that following a link from A -> B -> A would make the link to B stop working, when note A and B where on different notebooks

I'm not 100% sure of the root cause, but the reason is that when we render A and B for the first time there is a call to componentDidMount which calls note-screen-shared initState which loads the note by the props.noteId property

But when following the link from B -> A, now A doesn't reload the this.state.note, which would result on the navigation sending the user to the B notebook and trying to open the A note.

To fix this, I added a wrapper to the component that will be re-rendered every time the noteId changes, making it unnecessary the "hack" of redirecting to Notes and after a timeout redirect to Note.

Testing

Preparation

  1. Create notebook_a and notebook_b
  2. Create a note_a on notebook_a
  3. Create a note_b on notebook_b
  4. Add a link from note_a on note_b and vice versa

Testing

  1. Open note_a, follow link to note_b
  2. note_b should be open, follow link to note_a
  3. note_a should be opened
  4. Before this PR, trying to follow link to note_b wouldn't cause any changes
  5. After this PR, note_b should be opened

Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator left a comment

Choose a reason for hiding this comment

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

This looks good to me!

Edit: As discussed earlier, I'm changing the target branch from dev to release-3.0 to allow this change to be released before v3.1. (If this change should target v3.1, feel free to change the target branch back to dev!)

@personalizedrefrigerator personalizedrefrigerator changed the base branch from dev to release-3.0 July 15, 2024 16:27
@laurent22 laurent22 merged commit 70bfb26 into laurent22:release-3.0 Jul 16, 2024
10 checks passed
@pedr pedr deleted the fix-note-links-not-working branch July 16, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mobile: cannot open previously opened note if navigating with links
3 participants