Skip to content

[iOS, Mac] Exception should be thrown when shell item is null - #29464

Closed
HarishwaranVijayakumar wants to merge 6 commits into
dotnet:mainfrom
HarishwaranVijayakumar:fix-10041
Closed

[iOS, Mac] Exception should be thrown when shell item is null#29464
HarishwaranVijayakumar wants to merge 6 commits into
dotnet:mainfrom
HarishwaranVijayakumar:fix-10041

Conversation

@HarishwaranVijayakumar

@HarishwaranVijayakumar HarishwaranVijayakumar commented May 13, 2025

Copy link
Copy Markdown
Contributor

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Issue Details

  • When the Shell item is null, an exception is thrown on Windows and Android, but not on iOS and Mac.

Root Cause of the issue

  • Since the Shell does not contain any items, the Shell item is null. Instead of throwing an exception in iOS, the source code simply returns.

Description of Change

  • To ensure consistent behavior across all platforms, this pull request improves error handling in the ShellRenderer class for iOS and Mac. The SetupCurrentShellItem method has been updated to throw an InvalidOperationException when no current item is present."

For Windows, Issue will be fixed by this PR 28879

Tested the behaviour in the following platforms

  • Windows
  • Android
  • iOS
  • Mac

Output

Before Fix After Fix

@dotnet-policy-service dotnet-policy-service Bot added the community ✨ Community Contribution label May 13, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Hey there @@HarishwaranVijayakumar! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service Bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label May 13, 2025
if (Shell.CurrentItem == null)
{
return;
throw new InvalidOperationException($"Content not found for active {Shell}. Title: {Shell.Title}. Route: {Shell.Route}.");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is aligning the behavior with other platforms?

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.

@jsuarezruiz Yes on Android, InvalidOperationException is thrown. On windows, this pr #28879 will ensure the same behavior

@HarishwaranVijayakumar
HarishwaranVijayakumar marked this pull request as ready for review May 13, 2025 13:19
@HarishwaranVijayakumar
HarishwaranVijayakumar requested a review from a team as a code owner May 13, 2025 13:19
@rmarinho

Copy link
Copy Markdown
Member

What is the case where ShellItem is null?

@HarishwaranVijayakumar

Copy link
Copy Markdown
Contributor Author

@rmarinho If the ShellItem is null, an InvalidOperationException will be thrown

@mattleibow

Copy link
Copy Markdown
Member

The fact that this PR is for ios and mac and this other one is for windows: #28879

this means only android throws. Maybe android is wrong.

@mattleibow mattleibow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is what android does, for future me

if (ShellItem is null)
throw new InvalidOperationException("Active Shell Item not set. Have you added any Shell Items to your Shell?");
if (ShellItem.CurrentItem is null)
throw new InvalidOperationException($"Content not found for active {ShellItem}. Title: {ShellItem.Title}. Route: {ShellItem.Route}.");

@mattleibow mattleibow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This needs a test to be shared. Probably worth merging with #28879

@HarishwaranVijayakumar

Copy link
Copy Markdown
Contributor Author

Closing this PR, since the changes are merged into this PR #28879

@github-actions github-actions Bot locked and limited conversation to collaborators Aug 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration platform/ios platform/macos macOS / Mac Catalyst

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants