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

Add design time converters #19301

Merged
merged 2 commits into from
Dec 20, 2023
Merged

Add design time converters #19301

merged 2 commits into from
Dec 20, 2023

Conversation

etvorun
Copy link
Contributor

@etvorun etvorun commented Dec 7, 2023

Add design time converters to improve user experience when using XAML intellisense and hot reload.

@etvorun etvorun marked this pull request as ready for review December 8, 2023 01:27
@etvorun etvorun requested a review from a team as a code owner December 8, 2023 01:27
@etvorun etvorun requested review from hartez and jfversluis December 8, 2023 01:27
drasticactions
drasticactions previously approved these changes Dec 8, 2023
Copy link
Contributor

@drasticactions drasticactions left a comment

Choose a reason for hiding this comment

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

This looks good to me. The Unit Tests should cover the cases where if someone did update the underlying converter without updating the design time converter, causing it to break and force you to update both.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Generally looks good, adding tests where we had none before! 👍

@jsuarezruiz jsuarezruiz added the area-xaml XAML, CSS, Triggers, Behaviors label Dec 11, 2023
hasY = (xywh.Length == 2 || xywh.Length == 4) && double.TryParse(xywh[1], NumberStyles.Number, CultureInfo.InvariantCulture, out _);
hasW = xywh.Length == 4 && double.TryParse(xywh[2], NumberStyles.Number, CultureInfo.InvariantCulture, out _);
hasH = xywh.Length == 4 && double.TryParse(xywh[3], NumberStyles.Number, CultureInfo.InvariantCulture, out _);
if (!hasW && xywh.Length == 4 && string.Compare("AutoSize", xywh[2].Trim(), StringComparison.OrdinalIgnoreCase) == 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just thinking out loud: Would this work

Suggested change
if (!hasW && xywh.Length == 4 && string.Compare("AutoSize", xywh[2].Trim(), StringComparison.OrdinalIgnoreCase) == 0)
if (!hasW && xywh.Length == 4 && string.Compare("AutoSize", xywh[2].AsSpan().Trim(), StringComparison.OrdinalIgnoreCase) == 0)

?

I just rembember there was: dotnet/runtime#75452 and https://learn.microsoft.com/en-us/dotnet/api/system.memoryextensions.trim?view=net-7.0.

@rmarinho rmarinho merged commit e737057 into main Dec 20, 2023
47 checks passed
@rmarinho rmarinho deleted the dev/evgenyt/design_time_converters branch December 20, 2023 18:23
@github-actions github-actions bot locked and limited conversation to collaborators Jan 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants