Skip to content

Fixed entry text color [iOS] #20100

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

Merged
merged 10 commits into from
Apr 8, 2024
Merged

Conversation

kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Jan 23, 2024

Description of Change

Added MapFormatting() for MapText in Entry.iOS like it is done in EntryHandler.iOS

Issues Fixed

Fixes #19509
Fixes #19470
Fixes #19424

Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2024-01-27.at.13.44.26.mp4

@kubaflo kubaflo requested a review from a team as a code owner January 23, 2024 10:30
@ghost ghost added the community ✨ Community Contribution label Jan 23, 2024
@ghost
Copy link

ghost commented Jan 23, 2024

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

@jfversluis
Copy link
Member

You're on fire @kubaflo thank you!

@jfversluis
Copy link
Member

/azp run

@jfversluis jfversluis added platform/ios legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor labels Jan 23, 2024
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

Thanks for the contribution. I can help adding UI tests here.

@kubaflo
Copy link
Contributor Author

kubaflo commented Jan 23, 2024

You're on fire @kubaflo thank you!

I'm still learning the MAUI codebase, but I try to be helpful! 😅

@jsuarezruiz
Copy link
Contributor

Added UITest.

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@dartasen
Copy link
Contributor

@jsuarezruiz @jfversluis Any update on this ?

@jfversluis
Copy link
Member

jfversluis commented Mar 21, 2024

/azp run MAUI-UITests-public

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@shawncothran
Copy link

@PureWeen or @jsuarezruiz any plan to move this one forward soon?

@Zack-G-I-T
Copy link

Any update on when this will be merged?


private static void MapFormatting(IEntryHandler handler, IEntry entry)
{
handler.PlatformView?.UpdateMaxLength(entry);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of calling these directly can you propagate through the handler?

handler.UpdateValue(nameof(IEntry.MaxLength))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

Choose a reason for hiding this comment

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

thx!

@kubaflo kubaflo force-pushed the entry-text-color-fix branch from 6895f57 to 225ebcb Compare March 29, 2024 01:37
Copy link
Member

@jfversluis jfversluis left a comment

Choose a reason for hiding this comment

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

Also see a very lengthy discussion about this on the live stream here: https://www.youtube.com/watch?v=ERh9Ixz8f_Y

😄

{
base.OnAppearing();

await Task.Delay(1500);
Copy link
Member

Choose a reason for hiding this comment

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

What if we trigger the text change just with a button click instead of a delay? That might speed up things and is less arbitrary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did as you suggested and it does the job!

// Any text update requires that we update any attributed string formatting
handler.UpdateValue(nameof(IEntry.MaxLength));
handler.UpdateValue(nameof(IEntry.CharacterSpacing));
handler.UpdateValue(nameof(IEntry.HorizontalTextAlignment));
Copy link

Choose a reason for hiding this comment

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

I'm really new to the MAUI code (I really appreciate all your great work!), but I'm curious why we wouldn't have an handler.UpdateValue(nameof(IEntry.TextColor)); here? I'm curious how these length, spacing and alignment attributes fix the text color (from what I gather, it looks like perhaps we are almost "tricking" iOS into updating the field?). Thank you! 😃

@kubaflo
Copy link
Contributor Author

kubaflo commented Apr 4, 2024

Also see a very lengthy discussion about this on the live stream here: https://www.youtube.com/watch?v=ERh9Ixz8f_Y

😄

@jfversluis Oh no I missed that stream :/ I rewatched it though, and must admit it was a really good one that hurts me even more that I wasn't there live haha


// 2. Verify that the Entry bounded TextColor is correct (Green).
var color = App.FindElement("").GetText();
Assert.AreEqual("[Color: Red=0, Green=0.5019608, Blue=0, Alpha=1]", color);
Copy link
Member

Choose a reason for hiding this comment

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

TextColor won't propagate back up if it's changed unexpectedly on the platform

image

I think we'll need to test this through screen shot comparisons

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea

@PureWeen
Copy link
Member

PureWeen commented Apr 6, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Apr 7, 2024

Can u add the screen shots for all four platforms?

@PureWeen
Copy link
Member

PureWeen commented Apr 7, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@kubaflo kubaflo force-pushed the entry-text-color-fix branch from a195a09 to 82ad24c Compare April 8, 2024 10:12
@PureWeen
Copy link
Member

PureWeen commented Apr 8, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@ISSPRO-Eng
Copy link

Any updates on when this will be implemented?

@PureWeen PureWeen merged commit 998f884 into dotnet:main Apr 8, 2024
@ISSPRO-Eng
Copy link

How do we utilize this fix using the Nuget Package? Or will there be a release soon

@MartyIX
Copy link
Contributor

MartyIX commented Apr 9, 2024

You can try https://github.com/dotnet/maui/wiki/Nightly-Builds. I check when a nightly build was done and compare it to the moment when a PR was merged.

There was some issue when not every night, there was actually a new build but it should be fixed now. Probably still worth checking the date to be on the safe side.

@github-actions github-actions bot locked and limited conversation to collaborators May 10, 2024
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet