biolab / orange3-text

🍊 :page_facing_up: Text Mining add-on for Orange3
Other
127 stars 84 forks source link

Widget UI changes following new orange-widget-base release #624

Closed irgolic closed 3 years ago

irgolic commented 3 years ago

Following the merge and release of https://github.com/biolab/orange-widget-base/pull/130, widgets will look a bit different. All widgets should be checked and adjusted.

See https://github.com/biolab/orange-widget-base/wiki/Widget-UI

ajdapretnar commented 3 years ago

Issues:

A general tendency is for widgets to be taller than wider. Most, however, are same or better now. @irgolic How to fix these issues? Custom layouts, SizeHints?

irgolic commented 3 years ago

Preprocess Text: too wide, redundant space to the right of control area

Screen Shot 2021-03-05 at 12 00 06

Is it possible that one of the preprocessors is really elaborate and genuinely has such a wide sizeHint? If that's the case, set a sizeHint. You can put a sizeHint either on that one preprocessor's UI, the mainArea, or the widget window itself.

EDIT: The controlArea width looked weird so I opened the code:

self.controlArea.setFixedWidth(220)

Also, Preprocess seemingly is also really wide. I looked into it, and this was in the widget:

    def sizeHint(self):
        sh = super().sizeHint()
        return sh.expandedTo(QSize(sh.width() + 300, 500))

Deleting the above solves all issues. I've found that most widgets are weirdly written/have legacy workarounds like this. Not that it'd be any one person's fault, the above four lines show three different people in git blame. If you find yourself doing elaborate workarounds of the same nature in multiple widgets, let me know.

Topic Modelling: too tall, a bit wide, but less problematic overall. Main "issue" for me a boxes around method parameters which seem redundant (they say "Options", which is self-evident)

Screen Shot 2021-03-05 at 12 00 15

That's a really weird widget. Do the options automatically show/hide as radio buttons are pressed? If that's the case, put the three radio buttons to the top, and keep an Options box below them which changes its contents.

Statistics: a disaster, probably needs a rubber somewhere. Too tall, Apply should be centered. Also + and x icons are kind of cut (but this is an issue already on master).

Screen Shot 2021-03-05 at 12 00 35

want_main_area = False? Also, see Create Class. If that one's ugly too, let's talk about how we could rework these.

Concordance: should be wider

Screen Shot 2021-03-05 at 12 00 54

I might fix this one in orange-widget-base.

Document Map: too tall, too narrow (should be wider than taller)

Screen Shot 2021-03-05 at 12 00 43

If this is mainArea, set a sizeHint, or refactor it to controlArea. If it IS controlArea, then is want_main_area = True?

@irgolic How to fix these issues? Custom layouts, SizeHints?

Setting a sizeHint should be foolproof if it's statically sized. If the size depends on the data, you'll want to allow its sizeHint to propagate. Also keep in mind, due to this, widgets may open at different sizes depending on if you open it with a connected data signal or not.

Right now a controlArea + mainArea widget sets its sizeHint height depending on the controlArea+buttonArea's height (with a minimum of 500), and sets a width aiming to make the mainArea square-ish, a bit wider than a square. I'll change it to also take into account the mainArea's sizeHint, so something like Concordance doesn't happen, assuming that the table in Concordance does indeed expose a larger width/height than is observed.

ajdapretnar commented 3 years ago

Thanks, I will have a look! I did not realize the ugly formatting was a result of legacy code, but it does make sense.

ajdapretnar commented 3 years ago

Statistics already sets want_main_area = False, so I don't know what to do with it. The others I think I can fix.

ajdapretnar commented 3 years ago

Looking at it, Topic Modelling has a similar behaviour to Discretize. I am not sure I would extract options from under the methods. 🤔 I am not against it, just thinking it would be in line with other such widgets.

irgolic commented 3 years ago

Statistics already sets want_main_area = False, so I don't know what to do with it. The others I think I can fix.

The implementation should follow Create Class, right? Perhaps this is also relevant to https://github.com/biolab/orange3/issues/5285.

ajdapretnar commented 3 years ago

Create Class also open unnecessarily tall, but it is less noticeable as the widget has more controls.