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

The XyzMapper fields on each Handler type should be 'readonly' #4870

Closed
Eilon opened this issue Feb 23, 2022 · 3 comments
Closed

The XyzMapper fields on each Handler type should be 'readonly' #4870

Eilon opened this issue Feb 23, 2022 · 3 comments
Labels
legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@Eilon
Copy link
Member

Eilon commented Feb 23, 2022

Description

The various Mapper fields on the various control handlers should be made 'readonly' so they don't get overwritten.

Examples:
https://github.com/dotnet/maui/blob/main/src/Core/src/Handlers/Button/ButtonHandler.cs#L19-L41
https://github.com/dotnet/maui/blob/main/src/Core/src/Handlers/Editor/EditorHandler.cs#L16-L37

We noticed this in the BlazorWebView handler (probably because I copied the code from some other handler to begin with) and it seems to make sense to mark them readonly.

Tagging @PureWeen .

Steps to Reproduce

N/A

Version with bug

Unknown/Other (please specify)

Last version that worked well

Unknown/Other

Affected platforms

I was not able test on other platforms

Affected platform versions

N/A

Did you find any workaround?

No response

Relevant log output

No response

@Eilon Eilon added t/bug Something isn't working legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor labels Feb 23, 2022
@wenwen60 wenwen60 added the s/verified Verified / Reproducible Issue ready for Engineering Triage label Mar 11, 2022
@Redth Redth added this to the 6.0.300-rc.2 milestone Mar 22, 2022
@antonfirsov
Copy link
Member

@elion @PureWeen shouldn't they be get-only properties instead? It feels like more appropriate C# practice.

public static IPropertyMapper<IImage, IButtonHandler> ImageButtonMapper { get; } = new PropertyMapper<IImage, IButtonHandler>()
{
	[nameof(IImage.Source)] = MapImageSource
};

@PureWeen
Copy link
Member

Currently we're using the settable nature of these to create custom mappers inside Controls

EntryHandler.Mapper = ControlsEntryMapper;

If we make them readonly then we'll need to modify how we're doing this.

@Eilon
Copy link
Member Author

Eilon commented Apr 11, 2022

Closing because this is by design. We are fixing the Blazor stuff where we pre-emptively changed it.

@Eilon Eilon closed this as completed Apr 11, 2022
@ghost ghost locked as resolved and limited conversation to collaborators May 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants