pfalcon / picotui

Lightweight, pure-Python Text User Interface (TUI) widget toolkit with minimal dependencies. Dedicated to the Pycopy project.
https://github.com/pfalcon/pycopy
MIT License
811 stars 70 forks source link

Inconsistent accessors for ~~selection~~ widgets #13

Closed pfalcon closed 6 years ago

pfalcon commented 7 years ago

Currently there's:

Additionally:

That's pretty inconsistent. The towards-consistency approach is apparently to use "selected", as set by ItemSelWidget base class. But that doesn't cover WListBox case, so maybe should go straight to a method. And that doesn't cover setting the current selected item. A separate method or same method without/with param?

Also, the WCheckBox is an interesting case. It can be easily classified as a selection widget too, but does it make sense to follow the same API as for other, clearly multiselection widgets, or would that cross the border of being non-intuitive?

peterjschroeder commented 7 years ago

Selected seems to make sense for all of them except WCheckBox.

pfalcon commented 6 years ago

So, to decide how to resolve this right, need to consider what usecases are behind the needed functionality.

  1. The original picotui usecase was: implementing "mostly static" settings-like dialog. In this usecase, widgets are instantiated with the needed values, then user interacts with each widget individually (there's no/little inter-widget dependency), then at the end, we need to get final values of each widget. That's why picotui so far coded with widget values are passed in constructors.

  2. But picotui is intended to be a generic UI library, and usecase of dynamic setting and interaction among widgets is viable usecase. So, it should be possible to set widget value beyond initial construction, and get and set values in consistent way across widgets.

Consequently, proposal:

Add get() and set() method for each widget. These should be methods (not attributes) to provide enough abstraction for composite widgets (like popup above).

Now, these methods would return "the most basic" widget value, e.g. a selection index for selection widgets. Where needed, it can be considered to add get_text() convenience method to get textual value, but that's not conclusive so far.

pfalcon commented 6 years ago

That all sounds good so far, but "dynamically updated" widget usecase is more complex than that. Consider a list box for example - one of course want to get/set current selection index, but in general case, one may also want to set new items for the list.

So, get()/set() methods aren't enough to implement fully dynamic model, more methods may be required beyond that, e.g. set_items() for list widgets. It's unclear if that can be sandardized across widgets, or would be adhoc. The latter choice is to make for now apparently.

pfalcon commented 6 years ago

Ok, the first (likely not last) outcome of the refactoring: https://github.com/pfalcon/picotui/issues/25

pfalcon commented 6 years ago

Addressed in 1.0. See issues linked above for more information.