fix: ResizeObserver not adjusting after tab change #213
Conversation
WalkthroughThe pull request introduces modifications to the Changes
Sequence DiagramsequenceDiagram
participant User
participant TabberEvent
participant ResizeObserver
User->>TabberEvent: Click Tab Header
TabberEvent->>ResizeObserver: Unobserve Current Active Tab Panel
TabberEvent->>TabberEvent: Update Active Tab Panel
TabberEvent->>ResizeObserver: Observe New Active Tab Panel
Possibly related PRs
Poem
✨ Finishing Touches
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: 0
🧹 Nitpick comments (3)
modules/ext.tabberNeue/ext.tabberNeue.js (3)
284-286: LGTM! The changes fix the ResizeObserver issue.The implementation correctly manages the ResizeObserver by unobserving the old tab panel before observing the new one. This ensures proper resize tracking after tab changes.
However, consider adding error handling to make the code more resilient:
- resizeObserver.unobserve( this.activeTabpanel ); + if ( resizeObserver && this.activeTabpanel ) { + resizeObserver.unobserve( this.activeTabpanel ); + } this.activeTabpanel = TabberAction.getTabpanel( this.activeTab ); - resizeObserver.observe( this.activeTabpanel ); + if ( resizeObserver && this.activeTabpanel ) { + resizeObserver.observe( this.activeTabpanel ); + }
284-286: Consider refactoring resize observer management.The resize observer management is duplicated across
onHeaderClick,resume, andpausemethods. Consider extracting this logic into a dedicated method to improve maintainability.class TabberEvent { + /** + * Updates the observed tab panel in the resize observer. + * + * @param {Element} oldPanel - The panel to unobserve + * @param {Element} newPanel - The panel to observe + */ + updateObservedPanel( oldPanel, newPanel ) { + if ( !resizeObserver ) { + return; + } + if ( oldPanel ) { + resizeObserver.unobserve( oldPanel ); + } + if ( newPanel ) { + resizeObserver.observe( newPanel ); + } + } + onHeaderClick( e ) { const tab = e.target.closest( '.tabber__tab' ); if ( tab ) { e.preventDefault(); this.activeTab = tab; - resizeObserver.unobserve( this.activeTabpanel ); + const oldPanel = this.activeTabpanel; this.activeTabpanel = TabberAction.getTabpanel( this.activeTab ); - resizeObserver.observe( this.activeTabpanel ); + this.updateObservedPanel( oldPanel, this.activeTabpanel ); // ... rest of the code } } resume() { this.header.addEventListener( 'click', this.onHeaderClick ); this.tablist.addEventListener( 'scroll', this.onTablistScroll ); this.tablist.addEventListener( 'keydown', this.onTablistKeydown ); resizeObserver.observe( this.tablist ); - resizeObserver.observe( this.activeTabpanel ); + this.updateObservedPanel( null, this.activeTabpanel ); } pause() { this.header.removeEventListener( 'click', this.onHeaderClick ); this.tablist.removeEventListener( 'scroll', this.onTablistScroll ); this.tablist.removeEventListener( 'keydown', this.onTablistKeydown ); resizeObserver.unobserve( this.tablist ); - resizeObserver.unobserve( this.activeTabpanel ); + this.updateObservedPanel( this.activeTabpanel, null ); }Also applies to: 371-375, 391-395
Line range hint
13-13: Add ResizeObserver feature detection.The code assumes ResizeObserver is available but it might not be supported in all browsers. Consider adding feature detection:
-let resizeObserver; +let resizeObserver = null; async function load( tabberEls ) { const urlHash = window.location.hash.slice( 1 ); mw.loader.load( 'ext.tabberNeue.icons' ); - resizeObserver = new ResizeObserver( TabberAction.onResize ); + if ( typeof ResizeObserver !== 'undefined' ) { + resizeObserver = new ResizeObserver( TabberAction.onResize ); + } else { + mw.log.warn( 'ResizeObserver is not supported in this browser. Some features may not work correctly.' ); + }Also applies to: 506-507
|
Thank you so much for the fix. It is indeed the right place for the code |
Fix for #208.
Not sure if this the 100% correct place to adjust the resize observer, but it certainly works without problems in my testing.
Summary by CodeRabbit