Skip to content

fix(ios): prevent icon animation in TabGroup when closing modal windows#14412

Merged
hansemannn merged 6 commits intomainfrom
iOSTabbarIcon
Apr 26, 2026
Merged

fix(ios): prevent icon animation in TabGroup when closing modal windows#14412
hansemannn merged 6 commits intomainfrom
iOSTabbarIcon

Conversation

@m1ga
Copy link
Copy Markdown
Contributor

@m1ga m1ga commented Mar 24, 2026

var win1 = Ti.UI.createWindow();
var win2 = Ti.UI.createWindow();
win1.addEventListener("click", function() {
  var win3 = Ti.UI.createWindow();  
  win3.open({modal:true})
})
var tab1 = Ti.UI.createTab({window: win1,icon:"/images/appicon.png",title: 'Blue'});
var tab2 = Ti.UI.createTab({window: win2,icon:"/images/appicon.png",title: 'Red'});
tabGroup = Ti.UI.createTabGroup({tabs: [tab1, tab2]});
tabGroup.open();

Current issue:

iphone_tabgroup.mov

Icons will animate when you close the window. When removing that line it won't update the icons.

Also tested a normal app. Looks fine and shows the correct icons without issues.

Any thoughts if that code is still needed?

Comment thread iphone/Classes/TiUITabProxy.m Outdated
@m1ga m1ga marked this pull request as draft March 24, 2026 14:56
@m1ga m1ga marked this pull request as ready for review March 24, 2026 15:32
Comment thread iphone/Classes/TiUITabProxy.m Outdated
Comment on lines +83 to +98
UITraitCollection *currentTraitCollection = controller.traitCollection;

if (currentTraitCollection == nil) {
currentTraitCollection = rootWindow.hostingController.traitCollection;
}

if (currentTraitCollection == nil) {
return;
}

if ((lastTabBarTraitCollection != nil)
&& ![currentTraitCollection hasDifferentColorAppearanceComparedToTraitCollection:lastTabBarTraitCollection]
&& (currentTraitCollection.horizontalSizeClass == lastTabBarTraitCollection.horizontalSizeClass)
&& (currentTraitCollection.verticalSizeClass == lastTabBarTraitCollection.verticalSizeClass)) {
return;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason behind the trait collection check? If it's different, the underlaying issue should be in the presented modal state, not the return state.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prashantsaini1 Think you'll have to check that as that one was generated from your AI

Copy link
Copy Markdown
Contributor

@prashantsaini1 prashantsaini1 Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The didChangeTraitCollection is triggered only for ≥ 26, not below this version. It seems something must have changed internally in how the iOS handles it now for different classes (so we only compare it for horizontal + vertical sizes and color appearances for system dark/light modes).

@m1ga
Copy link
Copy Markdown
Contributor Author

m1ga commented Apr 26, 2026

made some changes after checking the file with Claude:

  • semantic guard in updateTabBarItem was only ever useful if updateTabBarItem could be called with an identical trait collection — but didChangeTraitCollection: already prevents that. For calls triggered by property setters (e.g. setIcon:, setTitle:), trait collection changes are irrelevant anyway, so the guard there was never meaningful to begin with.

  • All UI property accesses (controller.traitCollection, rootWindow.hostingController.traitCollection) are now inside the TiThreadPerformOnMainThread block, so they're always safe.

  • NO (non-blocking) is used instead of YES — there's no reason to block the notification thread waiting for the main thread here, and blocking could cause a deadlock if the notification ever fires from the main thread itself, since TiThreadPerformOnMainThread with YES on the main thread typically executes inline, but it's safer and consistent with how other notification handlers in Titanium work.

  • updateTabBarItem is called inside the block — it already has ENSURE_UI_THREAD_0_ARGS which becomes a no-op now since we're already on the main thread, which is the correct and efficient path.

@hansemannn
Copy link
Copy Markdown
Collaborator

@m1ga Were you able to test this already? If so, I'll give it another review and we're good to go!

@m1ga
Copy link
Copy Markdown
Contributor Author

m1ga commented Apr 26, 2026

yes, with the test example from the first post:

sim.mov
  • open/close modal,
  • dark/light mode switch
  • changing title

and with another real app where I've noticed the issue. All work fine

Copy link
Copy Markdown
Collaborator

@hansemannn hansemannn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thank you!

@hansemannn hansemannn merged commit 5588ea5 into main Apr 26, 2026
7 checks passed
@hansemannn hansemannn deleted the iOSTabbarIcon branch April 26, 2026 15:26
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.

3 participants