Skip to content

Needs override for remove_all_text from parent portal base to avoid a refresh exception #86

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

Closed
diego-wh opened this issue Jun 18, 2022 · 3 comments · Fixed by #92
Closed
Assignees

Comments

@diego-wh
Copy link

Portalbase has a function to remove all text (remove_all_text) this calls self.set_text to clear the labels.
Since magtag overrides this call to add the option to do a refresh ( auto_refresh: bool = True ) it would be nice to have an override of this function that takes the auto_refresh parameter. If we have more than a couple of labels (text fields) it is likely we will get the Refresh too soon exception.
The code change should be relatively simple (same as parent just passing the extra parameter to set_text). I am unsure if this is part of the scope for this library. I could not find an easy way to remove all objects from the display (maybe I am missing something), but that may be a suitable workaround.
I have been using a work around for my code

def remove_all_text():
    for i in range(count_labels):
        magtag.set_text("", auto_refresh=False, index = i)

where count_labels is a simple counter that increments every time I call add_text. but in this case I don't get the benefit of deleting the _text list ( self._text = [] ).

@diego-wh diego-wh changed the title Needs option to override remove_all_text from parent portal base to avoid a refresh loop Needs override for remove_all_text from parent portal base to avoid a refresh exception Jun 18, 2022
@ladyada
Copy link
Member

ladyada commented Jun 20, 2022

wanna submit a PR? @makermelissa can review when she returns in a couple weeks

@diego-wh
Copy link
Author

I could try to take a look at it over the weekend.

@makermelissa
Copy link
Collaborator

I don't have a MagTag with me, but it should just be a matter of adding an override function to this library.

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

Successfully merging a pull request may close this issue.

3 participants