peterbrittain / asciimatics

A cross platform package to do curses-like operations, plus higher level APIs and widgets to create text UIs and ASCII art animations
Apache License 2.0
3.62k stars 237 forks source link

Dwh298 #299

Closed dwhall closed 3 years ago

dwhall commented 3 years ago

Issues fixed by this PR

298

What does this implement/fix?

Allows setting the name attribute in Button's constructor.

Any other comments?

This only fixes half of the issue. I do not know how to fix the other half (its unit test remains in Error). This issue may apply to some other widgets, but I'm being lazy and only care about Button right now.

peterbrittain commented 3 years ago

Thanks for this. First question: why do you want to be able to name your buttons? They don't change their value when clicked... I guess this does allow you to find them, though. Of course you could just save off a reference when constructing them instead. Anyway... Tell me more about your use case.

dwhall commented 3 years ago

My use case is that I am trying to create an interactive status bar. I want to be able to dynamically change the text of the buttons that show information summaries. Clicking on the buttons activates a Frame to either show the information in greater detail or allow me to edit settings.

peterbrittain commented 3 years ago

Nice. Obviously, you don't need names for that as the name isn't what is displayed on the screen. It sounds like you want a new property to allow you to edit the text. That said, if you are using the name to find the button object, you'll also need this change to do that.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 97.159% when pulling 600fdaa27f401d8fcd86651fcdd435de02691a67 on dwhall:dwh298 into 7ae3758f9296570127d78dbc6e6b6837a2cf5539 on peterbrittain:master.

dwhall commented 3 years ago

I admit I was making an assumption that giving Button() a name would allow me to set its text via the Frame's .data property. Does this sound like a feature you would want?

peterbrittain commented 3 years ago

They are deliberately not the same thing. The name is the identifier for the programmer to use when setting/extracting values (and so should never change). The text is the string to display to the user (and so could change for all sorts of reasons - e.g. using a different language).

What I think you need is a text property. You could pick the logic from label.py to add the same function for buttons. Suggest you move the add_box logic to the setter for consistency (we want button.text = "foo" to have the same effect as passing "foo" as the display text to the Button constructor).

dwhall commented 3 years ago

Sorry, I meant giving the Button a name so I could use that name in a dict of name:values given to Frame .data to update the text of the Button.

Thanks for the tip, I'll give it a try on my fork.

I don't know github etiquette on who presses "Resolve conversation". I pressed it trying to see if that got rid of the "1 change requested", but it did not. Let me know if you need anything else changed. Otherwise, I'll let you re-review and merge/reject this PR.

peterbrittain commented 3 years ago

That's all good once you add the docs. If you want you can also add yourself to credits.py if you'd like to see your name in lights!

peterbrittain commented 3 years ago

Perfect! Many thanks.