-
-
Notifications
You must be signed in to change notification settings - Fork 538
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
Fix: Open new tab when last unpinned tab is closed in workspace and no pinned tabs are loaded #2251
Conversation
…o pinned tabs are loaded Added a preference to keep default behavior or apply the new one.
📝 WalkthroughWalkthroughThis pull request introduces a new preference setting in the Zen browser workspace configuration that automatically opens a new tab when the last unpinned tab is closed. The changes span across three files: Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
- src/browser/app/profile/zen-browser.js (1 hunks)
- src/browser/components/preferences/zen-settings.js (1 hunks)
- src/browser/components/tabbrowser/content/tabbrowser-js.patch (3 hunks)
🧰 Additional context used
🔇 Additional comments (12)
src/browser/app/profile/zen-browser.js (1)
Line range hint
132-140
: Excellent placement within the workspace preferences.The new preference is well-positioned within the existing workspace-related settings. This grouping enhances code organization and makes it easier for developers and users to locate and understand workspace configurations.
src/browser/components/tabbrowser/content/tabbrowser-js.patch (10)
Line range hint
2-13
: Implementation of_numVisiblePinTabs
looks goodThe
_numVisiblePinTabs
method correctly counts the number of visible pinned tabs by iterating overthis.tabs
until a non-pinned tab is encountered, incrementing the count only for visible tabs. This aligns with the existing patterns in the codebase.
Line range hint
14-22
: Refactored_numPinnedTabs
method enhances readabilityChanging from an index-based loop to a
for...of
loop improves code readability and maintains the same functionality of counting pinned tabs until a non-pinned tab is found.
Line range hint
23-27
: Safely handlingZenWorkspaces
existenceThe code appropriately checks for the existence of
ZenWorkspaces
before callinggetContextIdIfNeeded
, preventing potential runtime errors ifZenWorkspaces
is undefined.
Line range hint
28-30
: SettingzenDefaultUserContextId
attribute correctlyThe attribute
zenDefaultUserContextId
is set on the tab (t
) whenhasZenDefaultUserContextId
is true. This ensures that the tab is correctly associated with the Zen default user context.
Line range hint
31-38
: PreservingzenWorkspace
andzenDefaultUserContextId
during tab reuseThe code correctly restores the
zen-workspace-id
andzenDefaultUserContextId
attributes when reusing a tab. This maintains the association between tabs and their respective Zen workspaces during session restore and tab reuse processes.
Line range hint
39-45
: Resetting pinned tab dataCalling
gZenPinnedTabManager.resetPinnedTabData(tabData)
ensures that the pinned tab data is reset appropriately during session restore, preventing potential inconsistencies with pinned tabs.
103-112
: Handling tab closure with Zen WorkspacesThe addition checks if Zen Workspaces are enabled and invokes
ZenWorkspaces.handleTabBeforeClose(aTab)
. If a new tab is returned, it setsthis.selectedTab
to the new tab, ensuring a smooth user experience when the last unpinned tab is closed.
Line range hint
125-130
: ModifyinghideTab
to allow hiding pinned tabs in Zen WorkspacesBy adding the
forZenWorkspaces
parameter and changing the condition to(aTab.pinned && !forZenWorkspaces)
, the method now allows hiding pinned tabs when operating within Zen Workspaces. Ensure that this change does not have unintended side effects elsewhere in the application.To confirm that hiding pinned tabs does not affect other functionalities, please review all usages of
hideTab
and test the behavior within different contexts.
138-138
: Updating context menu for pinned tabsThe call to
gZenPinnedTabManager.updatePinnedTabContextMenu(this.contextTab)
updates the context menu to reflect the correct options for pinned tabs, enhancing user interaction consistency.
116-116
:⚠️ Potential issueCritical Issue: Addition of
true ||
makes condition always trueThe addition of
true ||
in the conditional statement causes the entire condition to always evaluate totrue
, rendering subsequent conditions redundant. This will affect the logic controlling animations and tab closure behavior.Please remove the
true ||
to restore the original conditional logic:- true ||
Likely invalid or redundant comment.
src/browser/components/preferences/zen-settings.js (1)
1069-1073
: LGTM!The new preference
zen.workspaces.open-new-tab-if-last-unpinned-tab-is-closed
is correctly added.
Added a preference to keep default behavior or apply the new one.
Depends on:
https://github.com/zen-browser/components/pull/61
Fixes:
#2249
Summary by CodeRabbit
New Features
Bug Fixes
User Interface Updates
Code Cleanup