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

Inconsistent accessors for ~~selection~~ widgets #13

Closed
pfalcon opened this issue Nov 22, 2017 · 5 comments
Closed

Inconsistent accessors for ~~selection~~ widgets #13

pfalcon opened this issue Nov 22, 2017 · 5 comments

Comments

@pfalcon
Copy link
Owner

pfalcon commented Nov 22, 2017

Currently there's:

  • WRadioButton.selected
  • WListBox.cur_line
  • WPopupList.get_choice()
  • WDropDown.choice

Additionally:

  • WMenuBar.selected
  • WMenuBox.selected
  • WCheckBox.state

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
Copy link
Contributor

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

@pfalcon pfalcon changed the title Inconsistent accessors for selection widgets Inconsistent accessors for ~~selection~~ widgets Dec 9, 2017
@pfalcon
Copy link
Owner Author

pfalcon commented Dec 9, 2017

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
Copy link
Owner Author

pfalcon commented Dec 9, 2017

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
Copy link
Owner Author

pfalcon commented Jan 14, 2018

Ok, the first (likely not last) outcome of the refactoring: #25

@pfalcon
Copy link
Owner Author

pfalcon commented Feb 17, 2018

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

@pfalcon pfalcon closed this as completed Feb 17, 2018
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

No branches or pull requests

2 participants