-
Notifications
You must be signed in to change notification settings - Fork 5
Feature/checkbox update #15
Feature/checkbox update #15
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me falta probar en la testapp, te aviso cuando lo revise.
Acordate de actualizar estas neuvas feature en la wiki: https://github.com/mercadolibre/fury_mobile-ios-ui/wiki/Checkbox
#pragma mark - Public Methods | ||
- (void)setEnabled:(BOOL)enabled Animated:(BOOL)animated | ||
{ | ||
if (self.userInteractionEnabled == enabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usaría un Bool propio del checkbox para mantener el estado enabled/disabled. Nadie te debería modificar el userInteractionEnabled pero podrían y el estado de la ui quedaría raro.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lo que me causa un toque de duda ahí es que si no estoy cambiando el userInteractionEnabled, es posible que la interacción con el componente siga siendo manejada desde BooleanWidget. Lo voy a probar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahí lo probé y parece andar bien. Voy a commitear un cambio con una property BOOL isEnabled
self.userInteractionEnabled = enabled; | ||
if (enabled) { | ||
if (self.isBooleanWidgetOn) { | ||
[self fillCheckBoxExternalFromColor:[UIColor ml_meli_mid_grey] ToColor:[UIColor ml_meli_blue] Animated:animated]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Movería los [UIColor ...] a funciones que devuelvan el color para cada estado (checkboxColorOn, checkboxColorOff y checkboxColorDisabled). Así se actualizan desde un solo lugar en caso de querer cambiar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claro, yo lo dejé así pero en el fondo estaba pensando que estaría bueno que haya algún config de colores en el futuro. Lo voy a armar con dos métodos así está un pasito más cerca a tener los colores configurables.
- (void)setOnBooleanWidgetAnimated:(BOOL)animated | ||
{ | ||
if (self.isBooleanWidgetOn) { | ||
if (self.isBooleanWidgetOn || !self.userInteractionEnabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acá también usaría un booleano interno no la userInterationEnabled.
[clearAnimation setAnimations:[NSArray arrayWithObjects:colorClearAnimation, opacityClearAnimation, nil]]; | ||
|
||
[self.checkBoxInternalLayer addAnimation:clearAnimation forKey:@"animateClear"]; | ||
if (!self.userInteractionEnabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mismo criterio, no usaría el userInteractionEnabled como flag.
…lor, enabledOffColor, and disabledColor
Codecov Report
@@ Coverage Diff @@
## develop #15 +/- ##
===========================================
+ Coverage 69.41% 69.53% +0.12%
===========================================
Files 47 47
Lines 3106 3089 -17
===========================================
- Hits 2156 2148 -8
+ Misses 950 941 -9 |
return CGPointMake(x, y); | ||
- (UIColor *)disabledColor | ||
{ | ||
return [UIColor ml_meli_light_grey]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Los colores desde el PR antes eran ml_meli_mid_grey y ml_meli_grey. Cambiaste por que van estos otros o fue un error al pasarlo a las funciones?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MLAlexIartsev El widget deshabilitado esta cambiando de estado igual y cuando lo volvés a habilitar se muestra con el estado cambiado.
Y por otro lado tendríamos que validar la animación del check al pasar de habilitado a deshabilitado y viceversa.
|
||
- (void)testEnableCheckBox | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Te quedo este test vacío.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uy, me pasa por dormido. Ahí muevo los checks y ordeno bien
Se limpió la clase MLCheckBox. Se hizo una unificación inicial de los métodos responsables por las animaciones para disminuir la repetición del código.
Se añadió el método público setEnabled:(BOOL)Animated:(BOOL) a MLBooleanWidget que habilita/deshabilita la interacción del usuario. Se agregó un override en MLCheckBox que además tiñe el checkbox de UIColor.ml_meli_mid_grey manteniéndolo en el estado de seleccionado/no seleccionado.
La función nueva se añadió a pedido de Ayelen Santamaría para Combo Recommendations.
Nuevos estados de CheckBox:
Checked & Disabled:
Unchecked & Disabled:
Efecto de Cambio:
Se puede ver el funcionamiento de los cambios de estado en la aplicación de prueba en New UI -> Widgets -> CheckBox, RadioButton & Switch