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

Fix: Open new tab when last unpinned tab is closed in workspace and no pinned tabs are loaded #2251

Conversation

kristijanribaric
Copy link
Contributor

@kristijanribaric kristijanribaric commented Oct 20, 2024

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

    • Introduced a preference to automatically open a new tab when the last unpinned tab is closed, enhancing browsing continuity.
    • Enhanced theme management with improved UI elements for better user interaction and customization.
  • Bug Fixes

    • Improved event handling and cleanup processes for marketplace and workspace settings.
  • User Interface Updates

    • Refined UI rendering for theme preferences, including detailed handling of dropdowns and checkboxes.
  • Code Cleanup

    • Various adjustments for consistency and clarity throughout the codebase.

…o pinned tabs are loaded

Added a preference to keep default behavior or apply the new one.
Copy link

coderabbitai bot commented Oct 20, 2024

📝 Walkthrough

Walkthrough

This 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: zen-browser.js, which adds the new preference; zen-settings.js, which incorporates the preference into the settings management; and tabbrowser.js, which enhances tab visibility and management logic. The modifications maintain existing functionalities while improving user experience and code clarity.

Changes

File Path Change Summary
src/browser/app/profile/zen-browser.js Added a new preference: pref('zen.workspaces.open-new-tab-if-last-unpinned-tab-is-closed', true);
src/browser/components/preferences/zen-settings.js - Added new preference to Preferences.addAll: id: 'zen.workspaces.open-new-tab-if-last-unpinned-tab-is-closed'.
- Enhanced event handling and theme management logic.
- Improved UI rendering for theme preferences.
- Code cleanup for consistency.
src/browser/components/tabbrowser/content/tabbrowser.js - Added method: get _numVisiblePinTabs().
- Updated method signature: hideTab(aTab, aSource, forZenWorkspaces = false).
- Enhanced handling of tab visibility and user context management.

Possibly related PRs

  • ✨ Added new disable all button #2196: The changes in this PR involve modifications to the zen-settings.js file, specifically adding a new preference related to workspace management, which directly aligns with the new preference introduced in the main PR for opening a new tab when the last unpinned tab is closed.

Suggested reviewers

  • mauro-balades

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Files that changed from the base of the PR and between 841261a and 15ddd0f.

📒 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 good

The _numVisiblePinTabs method correctly counts the number of visible pinned tabs by iterating over this.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 readability

Changing 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 handling ZenWorkspaces existence

The code appropriately checks for the existence of ZenWorkspaces before calling getContextIdIfNeeded, preventing potential runtime errors if ZenWorkspaces is undefined.


Line range hint 28-30: Setting zenDefaultUserContextId attribute correctly

The attribute zenDefaultUserContextId is set on the tab (t) when hasZenDefaultUserContextId is true. This ensures that the tab is correctly associated with the Zen default user context.


Line range hint 31-38: Preserving zenWorkspace and zenDefaultUserContextId during tab reuse

The code correctly restores the zen-workspace-id and zenDefaultUserContextId 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 data

Calling 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 Workspaces

The addition checks if Zen Workspaces are enabled and invokes ZenWorkspaces.handleTabBeforeClose(aTab). If a new tab is returned, it sets this.selectedTab to the new tab, ensuring a smooth user experience when the last unpinned tab is closed.


Line range hint 125-130: Modifying hideTab to allow hiding pinned tabs in Zen Workspaces

By 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 tabs

The 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 issue

Critical Issue: Addition of true || makes condition always true

The addition of true || in the conditional statement causes the entire condition to always evaluate to true, 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.

src/browser/app/profile/zen-browser.js Show resolved Hide resolved
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.

2 participants