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

CSS targeting via key does not work in context manager #89

Open
theodox opened this issue Aug 19, 2017 · 1 comment
Open

CSS targeting via key does not work in context manager #89

theodox opened this issue Aug 19, 2017 · 1 comment
Labels

Comments

@theodox
Copy link
Owner

theodox commented Aug 19, 2017

I just noticed that the CSS feature targeting keys is not working in 2.2. I think this probably broke with 2.0, since the problem is that CSS.applies is looking for keys -- but since 2.0 the keys on an mGui control are set retroactively, not during the __init__ of any particular widget.

So this does not work:

 example = CSS ('named_item', backgroundColor=(1,0,0)

with example: 
       with FillForm():
               named_item = Button('should be red')

but this still does

     example = CSS ('named_item', backgroundColor=(1,0,0)
     with FillForm() as outer:
          named_item = Button('should be red')

     example.apply_recursive(outer)

and so does this:

 example = CSS ('named_item', backgroundColor=(1,0,0)
 with FillForm():
        named_item = Button('should be red', css = example)

I'm not sure if its worth resurrecting this feature. It's supposed to be an analogy to the "id" selector in regular CSS. However since a CSS object can be passed directly to a constructor, user can work around it like that. alternatively we could add an explict 'selector' kwarg, but that seems like overkill. I don't make much use of the string id target feature, so I won't mind if we just remove the documentation that talks about it .

What do other people think?

@bob-white
Copy link
Collaborator

bob-white commented Mar 1, 2018

I've never really taken advantage of the CSS features enough to even notice the problem.
However this doesn't actually work for me:

example = CSS ('named_item', backgroundColor=(1,0,0))
with FillForm() as outer:
    named_item = Button('should be red')
 example.apply_recursive(outer)

This however does:

example = CSS ('should_be_red', backgroundColor=(1,0,0))
with FillForm() as outer:
    named_item = Button('should be red')
example.apply_recursive(outer)

Interestingly if I change Nested.add to:

def add(self, control, key=None):
    control.key = key or control.key
    self.named_children[control.key] = control
    if control not in self.controls:
        self.controls.append(control)

    control._parent = ref(self)

Such that it updates control.key, then the original example works.

However it does break down if you reuse a name in the parent's scope, as only the most recent binding with get updated, and the others will fall back on their default. But we already have that problem when it comes to adding children to the parent namespace as is.

I don't have a large battery of things to test against right now, but this change doesn't break any of the current examples.

Though none of this fixes the case of using the CSS class as a context manager.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants