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

Implement CharacterSpacing property in SearchBarHandlers #494

Merged
merged 6 commits into from
Mar 17, 2021
Merged

Implement CharacterSpacing property in SearchBarHandlers #494

merged 6 commits into from
Mar 17, 2021

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Mar 12, 2021 •
edited
Loading

Description of Change

Implement CharacterSpacing property in SearchBarHandlers.

Related with #401

Platforms Affected

  • Core
  • iOS
  • Android

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)
  • Targets a single property for a single control (or intertwined few properties)
  • Adds the property to the appropriate interface
  • Avoids any changes not essential to the handler property
  • Adds the mapping to the PropertyMapper in the handler
  • Adds the mapping method to the Android, iOS, and Standard aspects of the handler
  • Implements the actual property updates (usually in extension methods in the Platform section of Core)
  • Tags ported renderer methods with [PortHandler]
  • Adds an example of the property to the sample project (MainPage)
  • Adds the property to the stub class
  • Implements basic property tests in DeviceTests

mattleibow
mattleibow reviewed Mar 13, 2021
View reviewed changes
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

Very nice.

After merging in main, I noticed the tests were failing. Seemed to be related to the fact that there was a static field on the handler resulting in the same field being used for all tests.

src/Core/src/Handlers/SearchBar/SearchBarHandler.Android.cs Outdated Show resolved Hide resolved
src/Core/src/Handlers/SearchBar/SearchBarHandler.Android.cs Outdated Show resolved Hide resolved
src/Core/src/Handlers/SearchBar/SearchBarHandler.iOS.cs Outdated Show resolved Hide resolved
src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.Android.cs Outdated
});

Assert.Equal(xplatCharacterSpacing, values.ViewValue);
Assert.Equal(expectedValue, values.NativeViewValue, 4);
Copy link
Member

Choose a reason for hiding this comment

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

I had to add rounding in order to get the tests to pass. I picked 4 as that was what the "em" coefficient precision was.

Copy link
Member

Choose a reason for hiding this comment

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

As I type this, I feel my own reviewing self say this is probably meant to be a constant instead of a magic number...

Copy link
Member

Choose a reason for hiding this comment

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

Yup. Took 3 minutes for me to give in and make a constant.

src/Core/src/Platform/Android/ViewGroupExtensions.cs Outdated
{
public static IEnumerable<T> GetChildrenOfType<T>(this AViewGroup self) where T : AView
{
for (var i = 0; i < self.ChildCount; i++)
Copy link
Member

Choose a reason for hiding this comment

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

Not sure which is better, but iOS seems to follow a "wide" search and this is "deep". Probably not too essential, but just a difference.

@hartez hartez changed the base branch from main to handler-staging March 17, 2021 01:02
@hartez hartez force-pushed the searchbar-characterspacing branch from 46575ef to 1fc500f Compare March 17, 2021 02:14
@hartez hartez merged commit 18ed4c4 into handler-staging Mar 17, 2021
@hartez hartez deleted the searchbar-characterspacing branch March 17, 2021 02:14
hartez added a commit that referenced this pull request Mar 17, 2021
@jsuarezruiz @mattleibow @hartez
Implement CharacterSpacing property in SearchBarHandlers (#494)
7c3a94c 
* Implement CharacterSpacing in SearchBarHandlers

* Tests failing because of static

* Rounding for Android

* Fix iOS test

* There! No magic numbers!

* Fix rebase issuse and make SearchBar stuff work on iOS

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: E.Z. Hart <hartez@gmail.com>
@hartez hartez mentioned this pull request Mar 18, 2021
Merged
hartez added a commit that referenced this pull request Mar 18, 2021
@jsuarezruiz @mattleibow @hartez
Implement CharacterSpacing property in SearchBarHandlers (#494)
1d8b317 
* Implement CharacterSpacing in SearchBarHandlers

* Tests failing because of static

* Rounding for Android

* Fix iOS test

* There! No magic numbers!

* Fix rebase issuse and make SearchBar stuff work on iOS

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: E.Z. Hart <hartez@gmail.com>
hartez added a commit that referenced this pull request Mar 18, 2021
@hartez @jsuarezruiz
@hevey @mattleibow @AmrAlSayed0
Handler staging (#547)
25e6476 
* TimePicker Handlers (#454)

* TimePickerHandlers

* Undo changes in sample

* Add some nullability fixes

* Fix build error

* Changes from PR feedback

* Fixed build error

* Updated tests

* Roll time picker and popup into one control

* Move 24 hour view check to handler

* Pull NSLocale -> CultureInfo code out to common property

Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com>
Co-authored-by: E.Z. Hart <hartez@gmail.com>

* CheckBoxHandlers (#432)

* Rename ICheck -> ICheckBox, remove Color stuff (to be replaced with Brush
later), fix DeviceTests

* Implement CharacterSpacing property in EditorHandlers (#516)

* Fix broken test and incorrect CharacterSpacing on iOS

* Implement HorizontalTextAlignment property in EntryHandlers (#524)

* Better text on example

* Remove IView from ITextAlignment

* Remove IView from IPlaceholder

* Implement IsTextPredictionEnabled property on Editor (#515)

* Initial implementation

* Added Tests

* Added PortHandler Attributes

* Added Tests

* Handle turning text prediction back on

Co-authored-by: E.Z. Hart <hartez@gmail.com>

* Implement ReturnType property in EntryHandlers (#518)

* Implement CharacterSpacing property in SearchBarHandlers (#494)

* Implement CharacterSpacing in SearchBarHandlers

* Tests failing because of static

* Rounding for Android

* Fix iOS test

* There! No magic numbers!

* Fix rebase issuse and make SearchBar stuff work on iOS

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: E.Z. Hart <hartez@gmail.com>

* Stepper Handlers (#517)

* StepperHandlers

* Fixed Stepper handler broken tests

* Remove duplicate searchbar

* Add IStepper interface

Co-authored-by: E.Z. Hart <hartez@gmail.com>

* IStepper.Increment -> IStepper.Step

* Picker Handlers (#433)

* PickerHandlers

* Nullability fixes

* Updated Picker device tests

* New things in the tests!

* Removed unnecessary Android Api level validation

* Added Picker Items null validation in iOS PickerHandler

* Moved Picker Handler tests between different classes

* Renamed NativePicker to MauiPicker

* Removed unused code from iOS PickerExtensions

* Fix build error

* Fix build error

* Remove duplicate class

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: E.Z. Hart <hartez@gmail.com>

* ActivityIndicator Handlers (#417)

* ActivityIndicatorHandler

* Register IActivityIndicator

* Add comments to IActivityIndicator

* Fix build error

* Implement Font properties in SearchBarHandlers (#533)

* Speed up PR builds (#510)

Reduce the jobs and use the new pools

* implement Font properties in SearchBarHandlers

* Fix rebase issues

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: E.Z. Hart <hartez@gmail.com>

* Implemented LineHeight on Label (#538)

* Implemented LineHeight on Label (#368)

* Fix-up after rebase

Co-authored-by: E.Z. Hart <hartez@gmail.com>

* Fix broken label tests; add some missing label tests; attributed string cleanup

* Consolidate and rename extension method classes

* Fix class names and de-duplicate methods

* Centralize time extensions

* Fix mapping

Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
Co-authored-by: Glenn Hevey <glenn.hevey@gmail.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: Amr <hassaninamr@yahoo.com>
@ghost ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Jul 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 27, 2023
@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.
Reviewers

@mattleibow mattleibow mattleibow left review comments

Assignees
No one assigned
Labels
area-controls-editor Editor area-controls-entry Entry platform/android 🤖 platform/iOS 🍎
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jsuarezruiz @mattleibow @Eilon @hartez @samhouts