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

style(Pack(visibility=VISIBLE) does not work #951

Closed
2 tasks done
robinsiebler opened this issue Jun 23, 2020 · 12 comments
Closed
2 tasks done

style(Pack(visibility=VISIBLE) does not work #951

robinsiebler opened this issue Jun 23, 2020 · 12 comments
Labels
bug A crash or error in behavior.

Comments

@robinsiebler
Copy link

robinsiebler commented Jun 23, 2020

Expected Behavior

self.busy_label.style=Pack(visibility=VISIBLE) should work

Current Behavior

I had a label:
self.busy_label = toga.Label('Fetching data...Please wait...', style=Pack(visibility=HIDDEN))

Later I tried to hide it:
self.busy_label.style=Pack(visibility=VISIBLE)

This did not work. Resizing the window did not help.

This. did work:
self.busy_label.style.visibility = VISIBLE

Steps to reproduce

If you click the 2nd or 3rd buttons, everything works fine, but as soon as you click the 1st button, everything is broken.

import toga
from toga.style import Pack
from toga.constants import ROW, COLUMN, HIDDEN, VISIBLE, CENTER

class BugApp(toga.App):

    def startup(self):
        # Main window of the application with title and size
        self.main_window = toga.MainWindow(title=self.name, size=(500, 500))

        self.busy_label = toga.Label('Fetching data...Please wait...', style=Pack(visibility=HIDDEN))

        self.button1 = toga.Button(label='style=Pack(visibility=VISIBLE)', on_press=self.clicked)
        self.button2 = toga.Button(label='style.visibility = VISIBLE', on_press=self.clicked)
        self.button3 = toga.Button(label='Hide Label', on_press=self.clicked)
        button_box = toga.Box(children=[self.button1, self.button2, self.button3], style=Pack(direction=ROW, padding=20, alignment=CENTER))
        label_box = toga.Box(children=[self.busy_label], style=Pack(direction=COLUMN, padding=20, alignment=CENTER))
        box = toga.Box(children=[button_box, label_box], style=Pack(direction=COLUMN, padding=20))

        self.main_window.content = box
        self.main_window.show()

    def clicked(self, button):
        if 'Pack' in button.label:
            self.busy_label.style=Pack(visibility=VISIBLE)
        elif 'Hide' in button.label:
            self.busy_label.style.visibility = HIDDEN
        else:
            self.busy_label.style.visibility = VISIBLE

        self.busy_label.refresh()

def main():
    # App name and namespace
    return BugApp('BugApp', 'mybugapp')


if __name__ == '__main__':
    app = main()
    app.main_loop()

Your Environment

  • Python Version: 3.7.7

  • Operating System and Version (select from the following and list the specific version number; if your OS is not listed, list that as well)

    • macOS - version: 10.15.5
  • Toga Version: 0.2.15

  • Toga Target (the type of app you are trying to generate)

    • cocoa
@freakboy3742 freakboy3742 added bug A crash or error in behavior. up-for-grabs labels Jun 23, 2020
@freakboy3742
Copy link
Member

Thanks for the report! I'm guessing the underlying problem is that assigning the entire style isn't triggering the re-application of styles.

@Vipul-Cariappa
Copy link
Contributor

Vipul-Cariappa commented Jul 11, 2020

The label object does not have style attribute. Style attribute in inherited from widget object, which is further inherited from Node object (travertino package node.py file). Node object does not have a setter method for style attribute.
Therefore
self.busy_label.style=Pack(visibility=VISIBLE)
Does not work. The style attribute gets changed in above line of code, but the linking between the widget and style does not take place. i.e changes required in _impl and factory like variables does not take place.
Adding a setter for style attribute in Node object and refreshing it should work
(It did work when I tried; Windows Machine; Python version 3.7; Changes I made is give below in Node object).

@style.setter
def style(self, value):
    self._style = value.copy(self.applicator)
    self.intrinsic = self._style.IntrinsicSize()
    self.layout = self._style.Box(self)
    self.refresh()

The changes works if and only if the label is visible and button is clicked to make it hidden. To make a hidden label visible this setter method is not working. The below program works. But the above program, mentioned in the issue is not working

def startup():
    ...
    label = Label("Some Text!", style=Pack(visibility=VISIBLE))
    button = Button("Hide Label", on_click=button_handler)
    ...

    def button_handler(button, *args):
        label.style = Pack(visibility=HIDDEN)

I hope I have helped in solving the issue. I am beginner in python and would like to contribute.

@freakboy3742
Copy link
Member

@Vipul-Cariappa Definitely looks like you're on the right track. The fact that you've traced this back to the Travertino Node suggests the fix actually belongs there.

@Vipul-Cariappa
Copy link
Contributor

I managed to figure out the problem. This bug is occurring due to a if statement in Travertino, declaration.py BaseStyle object. (BaseStyle object is inherited by Pack object. )Line No 183 if value != getattr(self, '_%s' % name, initial): in validated_property class method.
The purpose of this if statement is to check if a specified style is already applied, if applied do nothing, if not apply the specified style. (In this case style means visibility) But when we change the style (Pack instance) attribute of a widget the value of the variables change but the changes are not applied. Removal of this specified if statement solves the problem but allows the programmer to apply the same style over and over again.

@freakboy3742
Copy link
Member

@Vipul-Cariappa That might resolve the problem, but it's not a viable fix in this case. The line you're referenced is an optimization - if width is already set to 200, and you set the width to 200, no calculation is required. The case that is a problem is when the entire style is changed, which is entirely different. In that case, the value isn't changing - because it's being set for the first time.

@swapniljha001
Copy link

What can I do to help resolve this issue?

@freakboy3742
Copy link
Member

@swapniljha001 We need to establish a patch that does trigger the application of the style change, but doesn't require removal of the overall optimization. I'm not sure if @Vipul-Cariappa is still looking at this problem (it's been a few months since the last update); any investigation/exploration you want to do would be helpful.

@freakboy3742
Copy link
Member

I'm going to close this as fixed; we now have a test suite that explicitly tests for visibility, and it seems to be working.

@eichin
Copy link

eichin commented Apr 23, 2024

Tracing down a more complicated case (where I can't create something hidden, but if I change extra_row.style.visibility (a Box containing some Labels and NumberInputs) from "hidden" to "visible" and back, it disappears as expected) I ran https://toga.readthedocs.io/en/stable/tutorial/tutorial-0.html on linux (ubuntu pre-24.04, toga.__version__ is 0.4.2) and added button.style.visibility = "hidden" after the button.style.flex assignment, and it did nothing. If I throw in a print(button.style), it does say

flex: 1.0; padding-bottom: 50; padding-left: 50; padding-right: 50; padding-top: 50; visibility: hidden

(If instead I put widget.style.visibility = "hidden" in the button_handler function in the example, the button disappears on-click.)

If the test mentioned above was core/tests/style/test_applicator.py:test_set_hidden, I'm not sure it's a convincing test - since the widget (Button in this case) has the correct style.visibility value, but it's still appearing on screen.

I couldn't find documentation for how visible/hidden were supposed to be used (and applicators don't seem to be in the API Reference at all?) so maybe this is the wrong approach - while I'd love detail on more generically dynamic content, for my purposes creating hidden elements and unveiling them later is fine.)

@freakboy3742
Copy link
Member

@eichin FY I'm not sure if this is the same issue you're seeing but #2447 is tracking a known issue with visibility under GTK; the act of fixing that issue in #2435 has revealed there are some co-morbid problems with display.

The key distinction between "visible=HIDDEN" and "display=NONE" is that HIDDEN means the layout will leave space for the widget as if it were there, but not actually display it; but display=NONE treats the widget as if it doesn't exist. What has become clear via #2447 is that this isn't being done completely reliably at present.

As far as the applicator API goes - it's not in the API docs because it's not part of the public API for Toga - you shouldn't be touching the applicator API directly. If you want to modify a style, you should be modifying the style attribute; that should propegate through the style object, and be reflected on the layout.

Applicators are part of Travertino. Travertino is the underlying API we're using to manage styles; it's provides the base classes for Pack, with the ultimate goal that it will serve as the base classes for Colosseum, a full implementation of CSS. The applicator is the mechanism by which a change in style is "applied" to the nodes in a layout tree.

@eichin
Copy link

eichin commented Apr 23, 2024

Thanks for clearing that up about the applicator API (the hazards of picking things up from reading code :-)

What I'm seeing with visibility doesn't seem gtk-specific, in that android (emulated and hardware) (mis)behaves exactly the same way as gtk does.

(I confirmed what #2435 says about toggling style.display not working at all, gtk or android; css-style display as you describe it would definitely be a better match for what I'm trying to do than visibility, though I noticed that the documentation at https://toga.readthedocs.io/en/latest/reference/style/pack.html#display says "Space will be allocated for the element as if it were there, but the element itself will not be visible." which isn't that. Perhaps once .display works I should ticket clarifying the docs?)

@freakboy3742
Copy link
Member

Agreed that clarification/correctly is definitely required. At this point, I'd say #2435 has stalled, as we haven't heard from the contributor for a while. If you're feeling enthusiastic, I think that PR isn't that far from the finish line - it's mostly missing tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A crash or error in behavior.
Projects
None yet
Development

No branches or pull requests

5 participants