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

[Android] Adding prop android_minWidth to Switch component #29059

Closed
wants to merge 10 commits into from

Conversation

fabOnReact
Copy link
Contributor

@fabOnReact fabOnReact commented Jun 4, 2020

Summary

This issue fixes #29047
Adding prop android_minWidth to Switch component.

Changelog

[Android] [Fixed] - Adding prop android_minWidth to Switch component

Test Plan

CLICK TO OPEN TESTS RESULTS

I will write separate pull request to update the docs on https://github.com/facebook/react-native-website

AFTER

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 4, 2020
@react-native-bot react-native-bot added Bug Platform: Android Android applications. labels Jun 4, 2020
@analysis-bot
Copy link

analysis-bot commented Jun 4, 2020

Platform Engine Arch Size (bytes) Diff
ios - universal 10,755,840 -29,968

Base commit: e6fc20e

@analysis-bot
Copy link

analysis-bot commented Jun 4, 2020

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 6,759,620 -4,492
android hermes armeabi-v7a 6,422,277 -4,612
android hermes x86 7,145,867 -4,554
android hermes x86_64 7,035,844 -4,519
android jsc arm64-v8a 8,933,278 -4,115
android jsc armeabi-v7a 8,588,353 -4,230
android jsc x86 8,762,661 -4,180
android jsc x86_64 9,338,264 -4,137

Base commit: e6fc20e

@dulmandakh
Copy link
Contributor

why not support minWidth in style?

@HectorRicardo
Copy link

@dulmandakh that could also work, but I guess it is because the minimum width is only supported on Android and not on iOS, so by naming the property androidMinWidth it is clear that it only applies to android

@fabOnReact
Copy link
Contributor Author

Thanks for the support @HectorRicardo

@dulmandakh

Thanks and Sorry, but I do not work on iOS. I can not tell if this is possible or not.
If you request, I will move this logic to the style prop.

TLDR
I believe It is possible to build this logic on iOS.
The iOS Contributor would have to set a check on the width style prop on iOS side and over-ride that value based on minWidth value.

I did a brief research, this are my inconclusive results on iOS:


on iOS we are using UISwitch

@implementation RCTSwitchComponentView {
UISwitch *_switchView;
}

as explained on stackoverflow

UISwitch inherits from UIView. The width/height could be changed using UIView transform

UISwitch *aSwitch = [[UISwitch alloc] initWithFrame:CGRectMake(120, 120, 51, 31)];
aSwitch.transform = CGAffineTransformMakeScale(2.0, 2.0);

Seems difficult to change the width without using trasforms as said here

image

I personally would not use transform for the width/height prop. I would use transform for the transforms. I would not give the user a workaround to set UISwitch width/height on iOS using transforms, as this may not work as he expects.

The iOS developer would have to find a way to set the UISwitch width/height and then change the logic to take in consideration the minWidth value.

I could not find existing iOS api to set the minWidth of a UISwitch, I did not make extensive research as I want to specialize on Android.

Thanks a lot.
I wish you a good day

@HectorRicardo
Copy link

I also did some research, and although I don't have any Apple device to test, I am pretty sure the default UISwitch is not customizable. I suggest we leave the androidMinWidth attribute outside of the style object.

@fabOnReact fabOnReact marked this pull request as draft September 16, 2020 18:23
@fabOnReact fabOnReact changed the title [Android] Adding prop androidMinWidth to Switch component [Android] Adding prop android_minWidth to Switch component Feb 3, 2021
@fabOnReact fabOnReact marked this pull request as ready for review February 3, 2021 10:46
@fabOnReact
Copy link
Contributor Author

fabOnReact commented Feb 3, 2021

why not support minWidth in style?

Thanks a lot @dulmandakh for your code review.

The main reason is that I am specializing on Android, Java and ReactNative/JS.
Currently I don't work with ObjectiveC and iOS.

It is harder for me to work with many different languages and tools.

Thanks a lot

@fabOnReact fabOnReact closed this Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot modify the width of Switch component
6 participants