Added ActivityIndicator implementation for iOS #2829
Conversation
Signed-off-by: Moises <moises.aranas@gmail.com>
Added changes file for PR Signed-off-by: Moises <moises.aranas@gmail.com>
Signed-off-by: Moises <moises.aranas@gmail.com>
Signed-off-by: Moises <moises.aranas@gmail.com>
| self.native = UIView.new() | ||
| self.native.addSubview(self._activity_indicator) | ||
| self.native.translatesAutoresizingMaskIntoConstraints = False | ||
| self.native.sizeToFit() |
There was a problem hiding this comment.
Is there a reason that both the UIView and UIActivatorView is needed? Is it not possible to use UIActivityIndicatorView directly as the native widget? If it isn't, there should be a docstring here describing the reason; if it is... then that's an easy cleanup :-)
There was a problem hiding this comment.
For this one I had to wrap it in a parent UIView, because for some reason the UIActivityIndicatorView shows up even when stopped, regardless of whether I set the hidesWhenStopped property to true - I'll add it in the docstrings in case someone figures out what is wrong, but for now this was the only way I could think of that was relatively clean
There was a problem hiding this comment.
@freakboy3742 I added some information about the issue in a comment here: 7d98567
There was a problem hiding this comment.
Ok - you successfully nerd sniped me here :-)
I've worked out what is going on. hidesWhenStopped is implemented as a wrapper around the literal setHidden() implementation - so stopping the spinner isn't just making the image disappear, it's literally hiding the widget.
The catch: when Toga starts up, it applies styles to all widgets - and one of the styles it applies is visibility. So - when you don't have the wrapper, Toga creates the widget, sets hiddenWhenStopped, and then sets the widget to be visible... which means you can then see it.
This doesn't happen when the widget is wrapped by the UIView because the hidden attribute of the container view is separate from the hidden attribute of the spinner itself.
So - the fix here is to override set_hidden() on the iOS ActivityIndicator so that it takes into account whether the spinner is running before applying visibility on the underlying widget.
This might have an impact on the testbed test - the probe evaluating visibility might also need to be adjusted.
There was a problem hiding this comment.
Thanks for the explanation! I was too stuck at thinking that it may be an issue with the bridge calls that I didn't think to look at the base widget 😅 I'll investigate, will have a look at the potential testbed changes as well
There was a problem hiding this comment.
I've just pushed the changes @freakboy3742 - it seems the testbed didn't require any changes because the activityindicator test does not check for the visibility when stopped. This will need updating, but if it's fine on your end I'd like to do that in a separate change as I might need to implement probes for the other platforms too (gtk and cocoa)
There was a problem hiding this comment.
I'm happy for the test improvement to be a separate PR.
Suggested changes from beeware#2829 Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
Updated activityindicator.rst Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
… bug with the initial hidden state. Signed-off-by: Moises <moises.aranas@gmail.com>
| self.native = UIView.new() | ||
| self.native.addSubview(self._activity_indicator) | ||
| self.native.translatesAutoresizingMaskIntoConstraints = False | ||
| self.native.sizeToFit() |
There was a problem hiding this comment.
Ok - you successfully nerd sniped me here :-)
I've worked out what is going on. hidesWhenStopped is implemented as a wrapper around the literal setHidden() implementation - so stopping the spinner isn't just making the image disappear, it's literally hiding the widget.
The catch: when Toga starts up, it applies styles to all widgets - and one of the styles it applies is visibility. So - when you don't have the wrapper, Toga creates the widget, sets hiddenWhenStopped, and then sets the widget to be visible... which means you can then see it.
This doesn't happen when the widget is wrapped by the UIView because the hidden attribute of the container view is separate from the hidden attribute of the spinner itself.
So - the fix here is to override set_hidden() on the iOS ActivityIndicator so that it takes into account whether the spinner is running before applying visibility on the underlying widget.
This might have an impact on the testbed test - the probe evaluating visibility might also need to be adjusted.
…verriding set_hidden Signed-off-by: Moises <moises.aranas@gmail.com>
…e class Signed-off-by: Moises <moises.aranas@gmail.com>
Signed-off-by: Moises <moises.aranas@gmail.com>
Signed-off-by: Moises <moises.aranas@gmail.com>
… to do delete and add in separate commits) Signed-off-by: Moises <moises.aranas@gmail.com>
There was a problem hiding this comment.
This looks great - thanks!
As you've noted inline, there's an opportunity for an improved test to validate the visibility of the spinner; that's something we can do as a separate PR (if only because it will involve touching every platform, so it will be more complex to develop).
As noted on Discord, the GTK failure in CI isn't your doing - that's because of an update to PyGObject that has a backwards incompatible change. We're likely going to land #2550 in the near future that will remove the need for gbulb; but as an interim measure, I've pushed an upper bound to the PyGobject pin to this PR; that way, CI can continue to pass for this and any other PRs that need to run before #2550 lands.
| self.native = UIView.new() | ||
| self.native.addSubview(self._activity_indicator) | ||
| self.native.translatesAutoresizingMaskIntoConstraints = False | ||
| self.native.sizeToFit() |
There was a problem hiding this comment.
I'm happy for the test improvement to be a separate PR.
This change adds the ActivityIndicator for iOS, using UIActivityIndicatorView
PR Checklist: