fix: modified fixed and unfixed logic, fixed #4640 #4709
Conversation
|
WalkthroughThe changes in this pull request enhance the Changes
Possibly related PRs
Suggested labels
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/stores/src/modules/tabbar.ts (2)
314-321: Consider translating comments to English for consistencyThe comments in this code segment are written in Chinese, whereas the rest of the codebase uses English for comments. For consistency and maintainability, it's recommended to use a single language throughout the codebase.
430-435: Consider translating comments to English for consistencyThe comments in this code segment are written in Chinese, whereas the rest of the codebase uses English for comments. For consistency and maintainability, it's recommended to use a single language throughout the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/stores/src/modules/tabbar.ts (2 hunks)
🧰 Additional context used
🔇 Additional comments (1)
packages/stores/src/modules/tabbar.ts (1)
430-435: Verify the sorting logic when unpinning tabsWhen unpinning a tab, the
newIndexis set toaffixTabs.length, which calculates the index for non-affix (unpinned) tabs. Ensure that this index correctly positions the unpinned tab among the non-affix tabs to maintain the intended tab order.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/stores/src/modules/tabbar.ts (2)
315-322: LGTM! Consider settingaffixTabOrderfor consistent sorting.The added code improves the pinning functionality by ensuring proper sorting of pinned tabs. However, as noted in the comment,
affixTabOrderis not set, which could lead to inconsistent sorting in the future.Consider setting
affixTabOrderwhen pinning a tab to ensure consistent sorting:tab.meta.affixTab = true; tab.meta.title = oldTab?.meta?.title as string; this.tabs.splice(index, 1, tab); +tab.meta.affixTabOrder = affixTabs.length; // Set affixTabOrder to the current number of pinned tabs
430-435: LGTM! Consider updatingaffixTabOrderfor remaining pinned tabs.The added code improves the unpinning functionality by ensuring proper sorting of tabs after unpinning. However, as noted in the comment,
affixTabOrderis not updated, which could lead to inconsistent sorting of remaining pinned tabs.Consider updating
affixTabOrderfor remaining pinned tabs after unpinning:tab.meta.affixTab = false; tab.meta.title = oldTab?.meta?.title as string; this.tabs.splice(index, 1, tab); + +// Update affixTabOrder for remaining pinned tabs +this.tabs.forEach((t, i) => { + if (t.meta.affixTab) { + t.meta.affixTabOrder = i; + } +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/stores/src/modules/tabbar.ts (2 hunks)
🧰 Additional context used
🔇 Additional comments (2)
packages/stores/src/modules/tabbar.ts (2)
Line range hint
391-398: LGTM! Please clarify the purpose of incrementingdragEndIndex.The
sortTabsmethod correctly implements the logic for reordering tabs. However, the purpose of incrementingdragEndIndexis not clear from the context.Could you please clarify the purpose of incrementing
dragEndIndex? Is it related to drag-and-drop functionality? If it's not necessary, consider removing it:async sortTabs(oldIndex: number, newIndex: number) { const currentTab = this.tabs[oldIndex]; if (!currentTab) { return; } this.tabs.splice(oldIndex, 1); this.tabs.splice(newIndex, 0, currentTab); - this.dragEndIndex = this.dragEndIndex + 1; }
Line range hint
1-568: Overall, good improvements to tab management functionality.The changes enhance the pinning, unpinning, and sorting of tabs. Consider implementing the suggested improvements for
affixTabOrdermanagement to ensure consistent sorting. Also, please clarify the purpose ofdragEndIndexin thesortTabsmethod.
… (vbenjs#4709) * fix: modified fixed and unfixed logic, fixed vbenjs#4640 * fix: modified fixed and unfixed logic, fixed vbenjs#4640
Description
标签页关闭逻辑修复，固定了一个标签页后关闭左右页签会发生关闭不了应该关闭的左右标签，取消固定标签会跳回原来位置，现在逻辑修复为跳回活动标签第一个，逻辑仿照谷歌浏览器。
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
These updates provide a more intuitive experience for users managing their tabs, ensuring a seamless organization of pinned content.