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.

Sign up for GitHub

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

Jump to bottom

Fix for odd Cursor behavior #7371

Merged
merged 27 commits into from
Jul 8, 2022
Merged

Fix for odd Cursor behavior #7371

merged 27 commits into from
Jul 8, 2022

Conversation

SotoiGhost
Copy link
Contributor

@SotoiGhost SotoiGhost commented May 20, 2022 •
edited
Loading

Description of Change

This PR brings the following fixes on iOS, Mac, and Android:

  • Text is no longer transformed when IsPassword is enabled
  • Cursor does not longer make orbital jumps to the beginning or the end of an Entry/Editor control when:
    • The text is transformed
    • A Converter is used
    • Or a combination of the above
  • Entry/Editor CursorPosition and SelectedLength properties are now in harmony with its Platform properties and viceversa
  • Entry/Editor CursorPosition and SelectedLength are no longer overridden at Init stage

For Windows:

  • Entry/Editor CursorPosition and SelectedLength properties are now limited to Text.Length
  • Enabled the Editor and Entry Core.DeviceTests.
  • Fixed a scenario where the cursor jumps to the beginning of the Entry/Editor when IsPassword is set
  • Fixed a NRE when Keyboard property is not set

Issues Fixed

@Eilon Eilon added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 20, 2022
@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@hartez
Copy link
Contributor

hartez commented Jun 14, 2022

@SotoiGhost This needs to be retargeted to the net6.0 branch if we're going to put it in sr2.

@SotoiGhost SotoiGhost changed the base branch from main to net6.0 June 15, 2022 18:36
@SotoiGhost SotoiGhost requested a review from a team as a code owner June 15, 2022 18:36
@Redth Redth requested a review from PureWeen June 15, 2022 18:51
@PureWeen PureWeen mentioned this pull request Jun 16, 2022
Open
@devonuto
Copy link

@SotoiGhost how is this going? This issue is a bit of a blocker for me. Is there anything we can do to help get it through?

Israel Soto and others added 17 commits July 7, 2022 16:42
@SotoiGhost
[Entry/iOS] Fixed odd cursor behavior when TextTransform is set
50fcf39
@SotoiGhost
[Entry/iOS] Fixed odd cursor behavior when a Converter is used
d73f390
@SotoiGhost
[Entry/iOS] Added null validation when calculating the cursor offset
c256507
@SotoiGhost
[Entry/iOS] Now TextTransform is ignored if IsPassword is set to true
8f73eec
@SotoiGhost
[Entry/Android] Ignore TextTransform if IsPassword is enabled
53b6359
@SotoiGhost
[Entry/Android] Fixed cursor odd behavior when using a Converter
d433320
@SotoiGhost
[Editor/iOS] Fixed odd cursor behavior when using TextTransform or a …
1fa342f 
…Converter
@SotoiGhost
[iOS/MacCatalyst] Fixed CursorPosition out of sync between PlatformVi…
08e37b1 
…ew and VirtualView

* Fixed for Entry and Editor control
* Enhanced logic to update the CursorPosition when setting a text
@SotoiGhost
[iOS] Fixed bad cursor calculation causing NRE
2e59d9d 
* Fixed odd cursor position value caused by text being tranformed by Converters
@SotoiGhost
[iOS] Fixed SelectionLength out of sync on Entry and Editor controls
4b85ef8
@SotoiGhost
[Android] Fixed SelectionLength out of sync on Entry and Editor controls
4d822e3
@SotoiGhost
[Windows] Moved the fix for the odd cursor behavior
28524c3 
* Moved to the Platform.TextBox extension methods, so this fix can be applied to any subclass of TextBox
* Fixed where the CursorPosition could become negative
@SotoiGhost
[Windows] Fixed CursorPosition and SelectionLength out of sync on the…
5255f2b 
… Editor control
@SotoiGhost
Making everything internal to avoid breaking public API changes
5ce62b5
@SotoiGhost
[Entry/iOS] Removed fix that handles CursorPosition and SelectionLeng…
e03d6d8 
…th correctly
@SotoiGhost
[Win] Limit the CursorPosition and SelectionLength to Text.Length
7b2bdd4 
* Add the default TextBox Keyboard when Editor.Keyboard is null
@SotoiGhost
[Editor] Added the Core.DeviceTests for Windows
95fdf01
SotoiGhost and others added 9 commits July 7, 2022 16:43
@SotoiGhost
[Win/Entry] Added Core.DeviceTests for Windows
78727aa
@SotoiGhost
[Android] Fixed some Core.DeviceTests
cc8ed6e
@SotoiGhost
[Android] Fixed CursorPosition and SelectionLength being overrided at…
1757900 
… init
@SotoiGhost
[iOS|Mac] Added some fixes for Entry and Editor controls
5e58cef 
* Now Entry's CursorPosition and SelectionLength are in sync with its Platform properties
* Fixed Entry and Editor CursorPosition and SelectionLength override at init
@SotoiGhost
Moved some CursorPosition and SelectionLength tests to Core.DeviceTests
c944eaf
@SotoiGhost
Moved Entry and Editor's CursorPosition and SelectionLength tests fro…
2a46981 
…m Core to Control project
@SotoiGhost
[iOS] Fixed an issue with Entry's CursorPosition and SelectionLength
c5a3d57 
* Fixed some Entry and Editor device tests
@SotoiGhost
[Android] Fixed an issue with Entry's CursorPosition and SelectionLength
a2e7db9
@SotoiGhost
[Win] Fixed an issue with Entry's CursorPosition and SelectionLength
db8eddb
@SotoiGhost
Copy link
Contributor Author

@devonuto Sorry for the late reply, I missed your post. This PR was ready yesterday and hope to be reviewed by the following week.

@SotoiGhost
Copy link
Contributor Author

The following tests are failing on CI but are unrelated to these PR changes:

  • UpdatingSourceUpdatesImageCorrectly

@devonuto
Copy link

devonuto commented Jul 8, 2022

@devonuto Sorry for the late reply, I missed your post. This PR was ready yesterday and hope to be reviewed by the following week.

No worries. Looks like it's still failing a couple of checks. Will that stop it?

hartez
hartez suggested changes Jul 8, 2022
View reviewed changes
Copy link
Contributor

@hartez hartez left a comment

Choose a reason for hiding this comment

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

Looks great, just a concern about the casting in the connect/disconnect methods.

src/Core/src/Handlers/Entry/EntryHandler.Android.cs Outdated

if (!_set)
// TODO: NET7 issoto - Remove the casting once we can set the TPlatformView generic type as MauiAppCompatEditText
((MauiAppCompatEditText)PlatformView).SelectionChanged += OnSelectionChanged;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think all these direct casts should be if(PlatformView is MauiAppCompatEditText editText); if someone has previously subclassed EntryHandler and overridden CreatePlatformView to use a different AppCompatEditText subclass, then the cast to MauiAppCompatEditText will fail at runtime.

@hartez hartez merged commit c6865a2 into dotnet:net6.0 Jul 8, 2022
@devonuto
Copy link

@hartez @SotoiGhost I take it this merge didn't make it into 6.0.7?

@hartez hartez mentioned this pull request Jan 20, 2023
Closed
This was referenced Jun 30, 2023
Closed
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
@samhouts samhouts added the fixed-in-6.0.419 Look for this fix in 6.0.419! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hartez hartez hartez approved these changes

@PureWeen PureWeen Awaiting requested review from PureWeen

Assignees
No one assigned
Labels
fixed-in-6.0.419 Look for this fix in 6.0.419! legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor platform/android 🤖 platform/iOS 🍎 platform/windows 🪟 t/bug Something isn't working
Projects
None yet
Milestone
No milestone
6 participants
@SotoiGhost @jsuarezruiz @hartez @devonuto @Eilon @samhouts