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

Hide/show widget [Core] [Cocoa] #184

Merged
merged 2 commits into from Jul 12, 2017

Conversation

Projects
None yet
3 participants
@Dayof
Copy link
Contributor

commented Jun 25, 2017

Resolves Issue #181

@phildini

This comment has been minimized.

Copy link
Member

commented Jun 26, 2017

Hey there! Would you consider this ready for review, or are you still working on it?

@Dayof

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2017

I think that can be reviewed. Later, this feature can be update with the addition of the constraints update on Cocoa.

@@ -331,3 +330,21 @@ def set_font(self, font):
:type font: :class:`toga.Font`
"""
self._set_font(font)

def hide(self, widget):

This comment has been minimized.

Copy link
@freakboy3742

freakboy3742 Jun 26, 2017

Member

Why does this need to take widget as an argument? Isn't self the widget that needs to be hidden?

This comment has been minimized.

Copy link
@Dayof

Dayof Jun 26, 2017

Author Contributor

The idea is to use like this : self.outer_box.hide(self.label_box), but yeah I can update to self.label_box.hide().

@@ -14,6 +14,18 @@ def _set_container(self, container):
self._constraints._container = container
self.rehint()

def _set_hidden(self, status, children):
for view in self._container._impl.subviews:

This comment has been minimized.

Copy link
@freakboy3742

freakboy3742 Jun 26, 2017

Member

Iterating over the list of children should be something common to all containers; could this be moved up to the interface layer?

This comment has been minimized.

Copy link
@Dayof

Dayof Jun 26, 2017

Author Contributor

Ok.

@@ -14,6 +14,18 @@ def _set_container(self, container):
self._constraints._container = container
self.rehint()

def _set_hidden(self, status, children):
for view in self._container._impl.subviews:
[view.setHidden_(status) for c in children if c._impl == view]

This comment has been minimized.

Copy link
@freakboy3742

freakboy3742 Jun 26, 2017

Member

This will remove the child remove the child from the view; but it won't modify any of the CSS geometry.

This comment has been minimized.

Copy link
@phildini

phildini Jun 26, 2017

Member

I'm also slightly confused why this is a list comprehension... using a list comprehension just for the side-effects seems odd to me.

While I'm here, where is setHidden_ defined? I couldn't find it in a grep of the codebase?

This comment has been minimized.

Copy link
@Dayof

Dayof Jun 27, 2017

Author Contributor

Yes, is not updating the super view geometry yet, I tryed to use one of the recommendation here "https://stackoverflow.com/questions/19561269/autolayout-with-hidden-uiviews" but is a problem to look more deeply.

@phildini

This comment has been minimized.

Copy link
Member

commented Jun 26, 2017

It would be really thrilling to see anything that looks like a test or example code for this.

@Dayof

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2017

I'll commit the new demands and post a code as an example. Thanks for the review @phildini and @freakboy3742 !

@freakboy3742

This comment has been minimized.

Copy link
Member

commented Jun 27, 2017

@phildini The work @Ocupe is doing for the GSoC is aimed at making the Toga interface testable; hopefully we'll be able to add tests in the very near future.

@Dayof

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2017

@Dayof

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2017

@freakboy3742 @phildini @eliasdorneles does this PR still with some reviews to do?

@freakboy3742
Copy link
Member

left a comment

🚢

@freakboy3742 freakboy3742 merged commit 1e32c3a into beeware:master Jul 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.