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

web: pinned tab fixes #7692

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

01zulfi
Copy link
Collaborator

@01zulfi 01zulfi commented Feb 27, 2025

  • fix pinned tab not working after page reload
  • fix note opening in new tab even if its tab is present when active note is pinned

Closes #7683

* fix pinned tab not working after page reload
* fix note opening in new tab even if its tab is present when active note is pinned

Signed-off-by: 01zulfi <85733202+01zulfi@users.noreply.github.com>
Comment on lines +662 to +665
/**
* Should be true if we want to open a new tab when the active tab is pinned
*/
considerPinnedTab?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This method, openSession, can be called in various situations. In these situations, activeTab doesn't have a consistent definition. For example, openSession is called here:

  1. Opening a note on click on the notes list. When the method is called here, activeTab is the tab that is currently active in the editor NOT the tab we're trying to open.
  2. Focusing a tab. When the method is called here, activeTab is the tab we're trying to open. If that tab is already pinned, I'll never be able to open the tab this way at all, it will always open a new tab.

The current logic to open in new tab for pinned active tab is flawed in this sense as openSession method doesn't have a way to differentiate between those two scenarios. I think the current approach where the caller of openSession decides to consider active tab is okay for now, the only other fix I could think of was maybe a refactor of several editor-store methods.

Do let me know what do you think about this

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.

Tab duplicated erroneously
2 participants