Fixed Shell Tab is still visible after set Tab.IsVisible to false #24999
Conversation
| if (renderer?.ViewController != SelectedViewController) | ||
|
|
||
| if (renderer is null) | ||
| return; |
There was a problem hiding this comment.
this is returning too soon. CurrentRenderer isn't updated
There was a problem hiding this comment.
The method RendererForShellContent(ShellSection shellSection) returns null if no matching renderer is found for the given shellSection. In such cases, setting CurrentRenderer to null is necessary to avoid exceptions, particularly when ViewController or other renderer-related operations are invoked. Returning early when the renderer is null ensures we don’t proceed with invalid references, preventing potential runtime exceptions during shell navigation.
I believe this is a necessary safeguard. Please let me know if there’s a better approach or if you have additional recommendations.
| @@ -432,7 +432,7 @@ void OnCurrentShellSectionPropertyChanged(object? sender, System.ComponentModel. | |||
| if (_mainLevelTabs == null) | |||
| return; | |||
|
|
|||
| var currentItem = VirtualView.CurrentItem.CurrentItem; | |||
| var currentItem = VirtualView.CurrentItem?.CurrentItem; | |||
There was a problem hiding this comment.
did we enable nullable here ?
There was a problem hiding this comment.
Yes, the nullable reference types are enabled here. After clearing CurrentItem, it reverts to its default value, which can indeed be null. To prevent potential exceptions, I will ensure that proper null checks are in place before accessing CurrentItem.
| @@ -270,7 +270,7 @@ void OnVisibleChildRemoved(Element child) | |||
| if (CurrentItem == child) | |||
| { | |||
| if (ShellItemController.GetItems().Count == 0) | |||
| ClearValue(CurrentItemProperty, specificity: SetterSpecificity.FromHandler); | |||
| ClearValue(CurrentItemProperty); | |||
There was a problem hiding this comment.
what's the rationale for dropping the specificity ?
There was a problem hiding this comment.
The rationale for dropping the specificity in this case is due to how values are set and managed. Initially, ClearValue(CurrentItemProperty, SetterSpecificity.FromHandler) was used to clear values set by a handler, such as FromHandler.
When a value is set manually or bound using SetValue, the specificity FromHandler is automatically replaced by ManualValueSetter, making the FromHandler value irrelevant. The existing logic in ClearValueCore retrieves the current setter key (e.g., ManualValueSetter or other), but if the value was manually set, passing FromHandler to ClearValue becomes redundant as this key is no longer present in the context values.
If FromHandler has not been overridden by another setter, the method clears the value based on the original key retrieved from the property.
maui/src/Controls/src/Core/BindableObject.cs
Lines 128 to 129 in 87253a5
If it has been overridden, the method automatically clears the value according to the current setter, ensuring that the value is cleared appropriately.
maui/src/Controls/src/Core/BindableObject.cs
Line 140 in 87253a5
It is not necessary to pass the FromHandler in the ClearValue. Could you please share your thoughts on this approach?
There was a problem hiding this comment.
of you clear the value set FromHandler, it should go back to the previous value with the highest specificity. isn't this what you want ?
There was a problem hiding this comment.
Since we are passing the FromHandler specificity, it only checks the values set by the handler. Therefore, the manually set value for the CurrentItem property is not cleared when setting visibility. Resetting to the highest specificity is not our intention. Instead, we clear the value without specifying specificity to ensure it is properly cleared.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
github-actions
bot
force-pushed
the
fix-8788
branch
from
5ae2069
to
31d4cb1
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
|
/rebase |
github-actions
bot
force-pushed
the
fix-8788
branch
from
31d4cb1
to
e2c17a5
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
github-actions
bot
force-pushed
the
fix-8788
branch
from
ac93872
to
801728b
Compare
Description of Change
When the visibility of the TabItem is changed dynamically, TabBar needs to be updated to reflect the TabBar items count, but hiding the visible TabItem updates the TabContent but doesn't automatically adjust the tab count.
Issues Fixed
Each time the DisplayedPage is modified,we have updated the TabBar items count through setupmenu().
Validated the behaviour in the following platforms
Fixes #8788
Fixes #23717
Fixes #23780
Before
Shell_before_fix.mp4
After
Shell_After_fix.mp4