[iOS] Fix Editor placeholder issues #15883
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsuarezruiz
added
t/bug
rachelkang
reviewed
Jul 3, 2023
Comment on lines
132
to
134
| placeholderLabel.TextInsets = new UIEdgeInsets(edgeInsets.Top, edgeInsets.Left + lineFragmentPadding, | ||
| edgeInsets.Bottom, edgeInsets.Right + lineFragmentPadding); | ||
|
|
There was a problem hiding this comment.
What was not working with the existing TextInsets that the new LayoutConstraints fixes?
There was a problem hiding this comment.
Ditto. Why did the old way not work? And if we need to use constraints, can we create them without using the VFL? That seems like an unnecessary performance hit for some fairly simple constraints.
There was a problem hiding this comment.
@rachelkang Was setting the LineFragmentPadding + edgeInsets.Right to the placeholder UILabel Width (that's why it was cut).
@hartez I recovered the approach used in XF, but you're right. I have applied changes to simplify it without using constraints etc.
What do the changes look like?. Instead of use the Frame, can apply changes to use TextInsets.
mattleibow
approved these changes
Jul 29, 2023
mattleibow
reviewed
Jul 29, 2023
There was a problem hiding this comment.
This looks good, but I am thinking we can also add a test for when the spacing changes. Can we verify that when the spacing changes the placeholder is updated.
And also a test to check the placeholder is mostly correct? So if you have a test that changes the editor width/height, does the placeholder also update?
I am thinking new tests:
- placeholder size/pos is correct on init
- placeholder character spacing is correct when editor spacing changes
- placeholder size/pos is correct when editor spacing changes
rmarinho
approved these changes
Jul 31, 2023
rmarinho
added a commit
that referenced
this pull request
Aug 3, 2023
* [create-pull-request] automated change (#16413) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * [Windows] Fix Border corners clipping issue (#14403) * Fix Border corners clipping mistake on Windows * Updated impl * Created method to avoid duplicated code * Revert uneccesary changes * Changes in clipping sizing * Fix Rui issue * Added device tests * Remove unnecessary changes --------- Co-authored-by: Javier Suárez <[email protected]> * Add ViewHandler Mapper docs (#16296) Co-authored-by: Juan Diego Herrera <[email protected]> * [create-pull-request] automated change (#16427) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Fix device tests to tun on arm64 (#16408) * Build on arm64 * magic * Only install on Arm64 machines * cleanup demands on the pipeline and add windows parameter * Print system arch * Fix getting OSArchitecture --------- Co-authored-by: Rui Marinho <[email protected]> * [uitests] Fix appium script (#16451) * Bump node version * Add more info to install script * Try clean modules before install * Uninstall before install * fix * Cleanup yaml * Write output of drivers * Try remove plugins before uninstall * [iOS] Fix Editor placeholder issues (#15883) * Fix iOS Editor placeholder size, characterspacing and fontsize issues * Added Device Test * Invalidate CharacterSpacing updating the placeholder * Updated Impl * [Essentials] Make PublisherName public on NET8 (#16454) * Fix EntryCellRenderer to use fromhandler for text changes (#16458) * [compatibility] Skip test on iOS (#16452) * [iOS] Re-enable editor tests (#16365) * Re-enable editor tests on iOS * Fix merge problem * [build] Remove the old step for vs insertion on release (#16457) * Reinstate IContentView and ILayout methods (#16411) * Reinstate IContentView and ILayout methods Fixes #16166 * Limit fake methods to NET Standard 2.0 * Reverse the polarity * Simplify * Work around compatibility implementations of ICV/ICPL methods on ScrollView * Use correct interface for Frame * [net8.0] Don't move to use arm yet here * [net8.0] Make UpdateBackground public (#16453) * [api] Make UpdateBackground public * Keep extensions internal * Fix issue setting ContentPage gradient background on iOS/Catalyst (#15832) * Fix provisioning script * [provisioning] Set Xcode path correctly for Xamarin Hosting --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Javier Suárez <[email protected]> Co-authored-by: Javier Suárez <[email protected]> Co-authored-by: Matthew Leibowitz <[email protected]> Co-authored-by: Juan Diego Herrera <[email protected]> Co-authored-by: Shane Neuville <[email protected]> Co-authored-by: E.Z. Hart <[email protected]>
Eilon
removed
the
legacy-area-controls
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
Fix Editor placeholder issues on iOS/Catalyst:
Added device tests and changes in the Editor sample page from the .NET MAUI Gallery (added samples using CharacterSpacing, Fonts etc).
Issues Fixed
Fixes #15792
Fixes #15750