Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove assembly scanning from VisualTypeConverter #5102

Conversation

PureWeen
Copy link
Member

@PureWeen PureWeen commented Mar 7, 2022

Description of Change

  • Remove Assembly Scanning from VisualTypeConverter
  • Leave attribute registration in place
  • The Converter now reads from the Registrar from any registered Visual Types. This means if the user has opted for assembly scanning with legacy renderers those will get picked up.
  • Adds builder.RegisterVisual inside the Microsoft.Maui.Controls.Compatibility.Hosting namespace so 3rd party libraries can register visuals as part of their builder code

Issues Fixed

Fixes #4937

@PureWeen PureWeen requested a review from eerhardt March 7, 2022 19:47
@PureWeen PureWeen marked this pull request as draft March 7, 2022 20:24
@PureWeen PureWeen requested a review from eerhardt March 7, 2022 22:16
@PureWeen PureWeen marked this pull request as ready for review March 7, 2022 22:16
Comment on lines 33 to 34
// Check for visual assembly attributes after scanning for IVisual Types
// this will let users replace the default visual names if they want to
Copy link
Member

Choose a reason for hiding this comment

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

This comment could use some updating, since we don't "scan for IVisual Types" anymore, so it isn't checked "after" anymore.

@@ -54,6 +55,8 @@ public void Register(Type tview, Type trender, Type[] supportedVisuals, short pr
}
else
visualRenderers[supportedVisuals[i]] = (trender, priority);

RegisterVisual(supportedVisuals[i]);
Copy link
Member

Choose a reason for hiding this comment

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

You are probably getting ILLink warnings here since the supportedVisuals[i] isn't annotated as PublicParameterlessConstructor.

Comment on lines +99 to +100
name = name.Substring(0, name.Length - 6);
fullName = fullName.Substring(0, fullName.Length - 6);
Copy link
Member

Choose a reason for hiding this comment

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

Can we use ReadOnlySpan<char> and Slice here instead? Then below, when doing the comparisons, use SequenceEqual instead of ==.

And also not creating a new string appending Visual below. Just slice off "Visual" from visualName.

string name = visualType.Name;
string fullName = visualType.FullName;

if (name.EndsWith("Visual", StringComparison.OrdinalIgnoreCase))
Copy link
Member

Choose a reason for hiding this comment

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

It's odd that we use IgnoreCase here, but don't do IgnoreCase below when comparing visualName == $"{name}Visual"

@PureWeen PureWeen closed this Mar 8, 2022
@samhouts samhouts added the area-core-hosting Extensions / Hosting / AppBuilder / Startup label Jul 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-core-hosting Extensions / Hosting / AppBuilder / Startup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants