feat: add experimental option for sidebar auto-centering#612
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an experimental configuration option to enable auto-centering of the active tab in the sidebar panel. Previously, the sidebar auto-scrolling behavior was removed (commit e41d14d), but this PR provides a toggle to restore that functionality for users who prefer it.
Changes:
- Added new configuration option
tab.autoCenterInSidebar(disabled by default) under experimental features - Modified scrolling logic in
menulist-tab.tsto check both popup context and user preference - Added UI elements and localization for the new option
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/config/config.ts | Added new boolean configuration option tab.autoCenterInSidebar with default value false |
| src/components/menulist-tab.ts | Refactored scrollIntoViewIfActive() to check both popup context and user preference before auto-scrolling |
| src/pages/options/options.html | Added checkbox input for the new auto-center option in experimental features section |
| src/pages/options/options.ts | Added event handlers and observers for the new configuration option |
| src/pages/options/options-i18n.ts | Added localization setup for the new option label |
| static/_locales/en/messages.json | Added English translation string for the new option |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private async scrollIntoViewIfActive(): Promise<void> { | ||
| const isPopup = document.body.classList.contains('popup'); | ||
| const autoCenter = await config['tab.autoCenterInSidebar'].getValue(); | ||
| if (isPopup || autoCenter) { | ||
| // Scroll the active tab into view at the center of the sidebar | ||
| // Using requestAnimationFrame to ensure DOM is ready | ||
| requestAnimationFrame(() => { | ||
| this.scrollIntoView({ behavior: 'smooth', block: 'center' }); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
This async function call on every tab render creates unnecessary performance overhead. The config value is cached by StorageConfigurationOption after the first access, so you should use the synchronous tryGetValueSync() method instead. This is particularly important since this code path is triggered for every active tab during rendering. Consider this pattern used elsewhere in the codebase (e.g., panel-windows.ts line 96):
private scrollIntoViewIfActive(): void {
const isPopup = document.body.classList.contains('popup');
const autoCenter = config['tab.autoCenterInSidebar'].tryGetValueSync() ?? false;
if (isPopup || autoCenter) {
requestAnimationFrame(() => {
this.scrollIntoView({ behavior: 'smooth', block: 'center' });
});
}
}
The cached value will be available after the config is initialized, and the default value of false is safe to use as a fallback.
2 participants
Description
This PR adds an experimental option to toggle the auto-centering behavior in the sidebar panel.
Following the recent change (commit e41d14d), the sidebar no longer auto-scrolls to the active tab. While this provides great operational stability for many, some power users with high tab counts still prefer the auto-centering navigation in the sidebar.
Key Changes
Auto-center active tab in sidebarunder "Experimental Features" (disabled by default).menulist-tab.tsto respect both the popup context and the user's preference.Verification
Reference:
I shared some thoughts on why this toggle is beneficial for power users here:
[URL_OF_MY_COMMENT]