[PATCH] Panel scrollbars + prettier scrollbars in dialogs

[kybl]

Hi, I'm coming with a patch with improving UI regarding scrollbars. It actually brings two things:

1) Improved scrollbar in a list box

The current scrollbar in a list box is just an asterisk. My proposal is to change them to be all more useful + nicer + consistent with scrollbars from other programs:

• instead of one asterisk character, the scrollbar is a proper line created with Unicode block characters (in ASCII mode, it is replaced by # char)
• the scroll box length is proportional to the window size and the number of items. The more we can see, the longer is the scroll box. For example, if there are 30 items and we can see 10 items, the scroll box will take 1/3 of the window height. This is consistent with how usual GUI programs show scrollbars.
• position of the current asterisk is based on the current cursor position. In this patch, the position of the scroll box is based on the window view position. This is also for consistency with other programs (and usability as well)
• for more smoothness, the scroll box uses half-box Unicode character as well. So in the one physical character, there could be a full box, half box, or no box at all.

2) Scrollbars in the main panels

This patch also brings a scrollbar on the right side, next to each main panel. The main purpose is better UX; one could quickly say if there are 20, 100 or 1000 files in the directory and the cursor is now.

Scrollbar has similar properties as a scrollbar in the list box, except the "grey" background created with Unicode block characters (it would not be so nice to not have the background there), and no half-boxes (because of the background). I was inspired by Far Manager here.

Scrollbars in the main panels can be turned off or on in the "Layout" preferences. Currently, it is on by default.

Others

From the technical point of view, it introduces a new widget WScrollbar.

See screenshot before:after here.

Opinions?

[kybl]

Patch

[kybl]

Comparison of the current state (top) and the new one (bottom)

[zaytsev]

I very much like the scrollbars in the panels! The look very nice and are useful to see at a glance how much files are there.

By contrast, the scrollbars of the listbox I don't think are an improvement (visually). Please check the screenshot of FAR, which I actually quite like. You will see that their scrollbars are almost like the current mc scrollbars, only then have an arguably nicer indicator - a dark block instead of star and light blocks as background. The arrows are positioned at the top - 1 and bottom + 1 (you moved them to top & bottom), and the scrollbar itself is on the dialog border, and not inside the dialog (as before). You also don't have light/dark blocks there.

Is there any reason that I don't understand as to why you didn't do the same as you did for panels? I thought this would have been really great...

[andrew_b]

Replying to kybl:

• instead of one asterisk character, the scrollbar is a proper line created with Unicode block characters

[...]

• for more smoothness, the scroll box uses half-box Unicode character as well. So in the one physical character, there could be a full box, half box, or no box at all.

Please avoid any hardcoded Unicode characters for decoration. Use skin engine instead.

2) Scrollbars in the main panels

#1483.

From the technical point of view, it introduces a new widget WScrollbar.

That's a correct way. But WScrollbar is a standalone widget. It's used with another standalone widget. Both those widgets should be joined to a group.

For example, WPanel should be reimplemented as a group with at least two widgets: WFileList and one of two WScrollbar(s). WListbox too: say WListView and one or two WScrollbar(s).

[kybl]

Different types of listbox scrollbars

[kybl]

Replying to zaytsev:

By contrast, the scrollbars of the listbox I don't think are an improvement (visually). Please check the screenshot of FAR, which I actually quite like. You will see that their scrollbars are almost like the current mc scrollbars, only then have an arguably nicer indicator - a dark block instead of star and light blocks as background. The arrows are positioned at the top - 1 and bottom + 1 (you moved them to top & bottom), and the scrollbar itself is on the dialog border, and not inside the dialog (as before). You also don't have light/dark blocks there.
Is there any reason that I don't understand as to why you didn't do the same as you did for panels? I thought this would have been really great...

Thanks for the comment! The reason why I made it in this way is Far Manager dialog has padding. So the scrollbar is not directly on the side, it is clear it is a scrollbar. In Midnight Commander, there is no window padding so the black scroll box can look weird (for example next to shadow).

However, it is quite a good idea to make the same style as in the panel. There is not possible to use granularity though but I like it more as well.

See the new screenshot: On the left is your proposal but with the current style. The center is the same with the same style as the panel (so actually exactly your proposal). On the right, there is the same as the center one but from top-1 to bottom+1. Personally, I like the center and right style.

[kybl]

Replying to andrew_b:

Replying to kybl:

• instead of one asterisk character, the scrollbar is a proper line created with Unicode block characters

[...]

• for more smoothness, the scroll box uses half-box Unicode character as well. So in the one physical character, there could be a full box, half box, or no box at all.

Please avoid any hardcoded Unicode characters for decoration. Use skin engine instead.

You are right. I'll do that.

2) Scrollbars in the main panels

#1483.

Whoa! That is a really old try. I hope I could finish this.

From the technical point of view, it introduces a new widget WScrollbar.

That's a correct way. But WScrollbar is a standalone widget. It's used with another standalone widget. Both those widgets should be joined to a group.

For example, WPanel should be reimplemented as a group with at least two widgets: WFileList and one of two WScrollbar(s). WListbox too: say WListView and one or two WScrollbar(s).

Yeah, that could be better. Not sure if I'm familiar enough how to do that but I can try :-)

[andrew_b]

Replying to kybl:

For example, WPanel should be reimplemented as a group with at least two widgets: WFileList and one of two WScrollbar(s). WListbox too: say WListView and one or two WScrollbar(s).

Yeah, that could be better. Not sure if I'm familiar enough how to do that but I can try :-)

As an exercise, you can play with the WChattrBoxes object (src/filemanager/chattr.c). WChattrBoxes is an a group of WCheck buttons, and I believe it's no so hard to add a scrollbar there.

[ossi]

the "displaced" scroll bar looks really weird.
but it's true that the one in the frame doesn't compose well with the shadow. so instead of the full block you could try left-three-quarters (0x258A) (and inverse lower-one-quarter (0x2582) at the bottom, if used anywhere).

don't put the arrows outside the area the scrollbar applies to (as you did in the panels) - that looks weird and unnecessarily interferes with the frame drawing.

btw, at least redhat-based distros have been carrying some scrollbars patch for a very long time.

[zaytsev]

Visually, I really like the center version. I think it looks totally awesome. But now I understand your logic. Maybe you could indeed try the suggestion by ossi - would be interesting to see if it looks better or not?

And in as far as panels are concerned, yes, the bottom part overlaps with the widget and it feels weird, I didn't notice this initially, but the top one I think is OK and looks good?

[kybl]

Scrollbar with 3/4 chars

[kybl]

Thanks for all the comments. I tried the ossi's idea with 3/4 but it seems weird because it is misplaced (see screenshot; not sure if I understood right the idea).

don't put the arrows outside the area the scrollbar applies to (as you did in the panels) - that looks weird and unnecessarily interferes with the frame drawing.

Ok, I did not agree initially but when I'm looking onto it more, I agree :-)

Visually, I really like the center version.

It seems it is the winner of this mini-poll :-) I'm going to implement this version.

[ossi]

yeah, you're right - the arrows emphasize the asymmetry to an unbearable level. you could try 0x275A instead and see whether it's wide enough in a selection of fonts.
one could also try entirely different shapes, e.g. 0x2666.

another way to approach this is to fix the shadow problem at the root: by adding padding. there isn't really a reason for the combo box popups to have none - in fact, the history popups in the various dialogs look kinda jarring due to the inconsistency once i start paying attention.

[psprint]

Skin support for arrow and other chars. Implement the middle type from the example png.

[psprint]

I'll be continuing the patch on kybl's behalf. Here's what I've added to it:

• skin support for the arrows and other chars (6 new optional types with unicode/ascii(uglymode) defaults: "uparrow", "uparrowempty", "dnarrow", "dnarrowempty", "box", "boxlight"),
• implemented the middle possibility from the example PNG anothertypes.png.

See patch and screenshot. What do you think?

andrew_b: as for the WGroup-based implementation, your intent is to:

• when creating the listbox (listbox_new) create WGroup instead,
• and add two widgets to it: WListbox and WScrollbar

?

I'm afraid that this will complicate the code - every WListbox field access (like l->top) will be have to be changed to access one of the WGroup's g->widgets... Could this be avoided? And some simpler implementation be considered?

[ossi]

cool!
a few random observations:

• you have a copy-pasto in lib/skin/lines.c:76
• don't name the theme items by the shape, but by their function - scroll_bar and scroll_slider in particular (i won't argue about the arrows, but maybe somebody has a better idea for that as well)
• it strikes me as a bad idea to include unicode into the source code. better use \uXXXX or even the utf-8 equivalents.
• the patch should be split, in particular the theming and the scrollbar addition to the panels are semantically separate.
• you have some whitespace issues, in particular around ?: constructs relating ugly drawing, and asterisks in function prototypes.

to enable my proposal of using entirely different shapes for the slider, more flexibility would be required: a toggle to enforce that the slider is always exactly one cell, and, to go completely overboard, splitting the slider into top, middle, and bottom sections. i don't really care if anyone implements this, but maybe you feel like it.

[andrew_b]

Pathch should be split:

• a full-functional WScrollbar implementation without applying to any widget (listbox, file panel, etc);
• applying of WScrollbar to WListbox;
• move applying of WScrollbar to WPanel to #1483.

Scrollbar shouldn't be just an indicator that shows a current position in listbox. Scrollbar should handle mouse events in order to change the position.

Replying to psprint:

I'll be continuing the patch on kybl's behalf. Here's what I've added to it:

• skin support for the arrows and other chars (6 new optional types with unicode/ascii(uglymode) defaults: "uparrow", "uparrowempty", "dnarrow", "dnarrowempty", "box", "boxlight"),

Implementation is incorrect. Scrollbar itself isn't a frame and scrollbar chars shouldn't be mixed with frame ones (see #3146 also).

As written comment:2, code should not contain any hardcoded Unicode characters for scrollbar. Skin engine should be used instead.

andrew_b: as for the WGroup-based implementation, your intent is to:

• when creating the listbox (listbox_new) create WGroup instead,
• and add two widgets to it: WListbox and WScrollbar

?

I'm afraid that this will complicate the code - every WListbox field access (like l->top) will be have to be changed to access one of the WGroup's g->widgets... Could this be avoided? And some simpler implementation be considered?

"Quick, simple, and somehow" work isn't our way.

A group of listbox and scrollbar has following advantages:

• this group looks as "solid" widget outside;
• z-order within group is kept and group is (re)drawn correctly automatically;
• scrollbar position and size are changed/kept automatically too.

[psprint]

I tried to convert the code to WGroup-based architecture, but the code turned out to be slightly too complex. I've had to come up with a special WListbox casting macro:

+#define LISTBOX_(x) ((WListbox_ *)(g_list_next(GROUP((WListbox *)(x))->widgets)->data))

and then apply it on the code in listbox.c... It turned out to be too invasive change, the patch is long and problematic (it has a bug so that the listbox size is set to 2x2), you can see this in the attached diff. Could the idea for WGroup-using implementation be think again? It doesn't seem to be the right architecture here, it seems more to be used in a situation where the widget is really just aggregating other widgets.

[ossi]

the code turned out to be slightly too complex.

then you're presumably doing it wrong. i mean, what andrew is asking for is a bog-standard architecture employed by just about every gui toolkit. draw some inspiration from qt or gtk.

[psprint]

The problem is that lisbox.c wants the actual WListbox, not the new WGroup aggregate, so this requires that each public function uses the fancy LISTBOX macro to dig it out from the WGroup. Private functions take the proper WListbox so no change in them. Now, this requires 2 lines of extra code in each public function (WListbox_ *l; l = LISTBOX_(w);) plus a reverse macro for calls to public function from static ones, or a function stubs/proxies. The later is better as it doesn't pollute listbox with the knowledge of existence of something that is called WListbox but isn't being one.

That pollution seems to be the main issue here, but the new file with listbox public functions mimicked isn't nice too.

[psprint]

Could someone answer?

[psprint]

Any answer? I'm coding this with financial support so I have to finish this but not with a wrong architecture.. And a proper scrollbar is something strong wanted, isn't it?

[ossi]

don't try to minimize the scope of the patch, but think of what would be the *right* design, and refactor towards that, if necessary in multiple steps. maybe you need more type adaptation, maybe some model-view separation.

[psprint]

Model view separation? Are you serious? Such engineering overkill for such a simple patch?

A question that has no answer: tell how would you avoid 2 lines of code added to each public function that would dig out the actual list box from WGroup. You cannot. And that's a bad design because it pollutes inner widget with knowledge of the container.

But maybe I'm wrong. If yes then feel free to direct me and I'll write the code.

[ossi]

you don't have to completely overkill it. you could, for example, alias some deeply nested structure inside the higher-level container, be it via a typed copy or #define magic.

[psprint]

The define magic is what I did with the LISTBOX_ macro. And still the problem of extracting the proper WListbox to access it within the public functions exists. Could you enlighten me on the typed copy? What do you mean by it?

I see that the WGroup architecture is completely wrong to conjoin WListbox and WScrollbox. Polluting the whole WListbox set of public methods with knowledge of outside object just to have the *impression* of automation of MSG_DRAW message is really just wrong. This isn't a situation of WDialog aggregating inner widgets. It's a situation of a FACADE to composited widgets, and this is an overkill to do so just to have the nice MSG_DRAW propagating automatically to inner widgets, because it requires what a typical facade requires: a complete collection of stubs forwarding public calls.

[ossi]

instead of the casting macros, you can #define fake members that forward to the actual nested members (of course, they'd have to have globally unambiguous identifiers). whether that's nicer is debatable.

but anyway, as i already said, you can actually keep typed copies of the widget pointers in the top-level widget, rather than going through the generic subwidget list each time.

also, give that thing a proper name, like ListScrollBox. or the other way round, and call the nested class SimpleListBox.

i can't judge whether this is really worth it, but if you think it's not, you should make a somewhat more convincing attempt at refuting the need for more automatic MSG_DRAW handling.

[psprint]

Replying to ossi:

i can't judge whether this is really worth it, but if you think it's not, you should make a somewhat more convincing attempt at refuting the need for more automatic MSG_DRAW handling.

As you've maybe noted, all this effort is to make MSG_DRAW automatic. That's the only goal – to have that nice effect that two widgets are automatically drawn after routing the draw request from the container. However, is it worth it, and also is it proper architecture? As I already noted, the design pattern isn't a Compositor (like WDialog is) but a Facade. This means that almost empty stubs forwarding to inner widgets (inline functions or macros) are needed. That price is IMO to high to pay just for the nice MSG_DRAW forwarding.

I was also into that auto-draw effect, it's a nice idea, to just put 2 widgets into WGroup and have the drawing done automatically, however, as it revealed to be a Facade and not simple Compositor, I think that the idea should be dropped. What's still possible to do is extending WListbox and putting new fields of WScrollbox into the new class. That would be a proper engineering, with the public WListbox methods working out of the box without any Facade forwarding stubs. However, I'm not sure if it would be better than the current arch of separate WScrollbox. Let andrew_b or you ossi decide. Which one to implement?

[ossi]

my point is that you don't really need a full facade - just make the simple list box a public member of the scrollable one. then accesses to the nested members still require an indirection, but no casting magic.
but really, about how many relevant members are we talking here?

don't name the class WScrollbox, as that's too generic (an editor window would kinda fit that name).

i can't decide anything, as i'm not an mc maintainer. my role is advisory.

[psprint]

Hi,
I thought that I'll revive the patch. I think that the initial patch architecture is a correct one. Could you reconsider? MC with the nice scroll bars is a much prettier one, more than with the shadows, whose architecture is disputable.

[zaytsev]

So by "reviving" you mean have another go at trying to get it accepted as is? I'm afraid this won't work.

I very much want to have this feature, but we want it to be done properly. Did you understand what Andrew wants and how to do it? Ossi tried to guide you in the right direction...

[psprint]

The "right dirrection" is a facade, which is a terrible design, because I've tried to iimplement it this way, and got hit by it's terribleness… Therefore, I expect andrew to reconsider, that's all.

[psprint]

So OK, could you explain to me how this should be implemented? I might misunderstood it. By what it said to me, andrew wants the implementation to use WGroup to collect WListBox and WScrollBox, to have them drawn at a single MSG_DRAW. However, it turned out to be nearly impossible, because WGroup doesn't have WListBox interface, which would then lead to a Facade design pattern, with many problems like digging out the WListBox from the WGroup in each forwarding-function stub. Because the new WListBox would need to have the interface of the old list box.

[ossi]

don't ask the same questions and make the same claims again, but go through the answers you already got, one by one, until you understand it well enough to implement it, to ask further questions, or to show in a convincing way that the suggestion just isn't that great. then someone may feel motivated to help you out more with the job you signed up for yourself.

[psprint]

I've already shown why the suggestion isn't that great, what's missing is your good will. I will continue in my efforts to make the patch be accepted, because of my own good will, which I have to dig out extras in order to save this patch and feature – i.e. for midnight commander project good. I'm thinking about visualizing the Facade design pattern's problems in this suggestion, but then again, I suspect that malice will make the presentation unnoticed, just like that…

[zaytsev]

Invoking the notions of good will and malice is quite pathetic. Life is a lot simpler than that. We have very little time, and you code is no good. We are trying to help you with the time we have, but you are not getting it. That's totally fine, but then you somehow come to expect that we would keep hammering you until you make it, or maybe even just write it for you. Sure, it would be good for you, if you end up learning something from it, and even for the project, because this feature will be implemented. But the time is not there... so unfortunately it ain't gonna happen this way.

[psprint]

Invoking the notions of 'pathetic' is malice. I provided the your-way patch to show you how bad idea it is. The code in the original patch is beautiful compared to it. All this comes from a little-depth idea of andrewb to have the single MSG_DRAW receptor. As you can see, the roles revert to me providing the correct code, and your propaganda of a bad one.

PS. It should be stated, that I'm not the author of the original patch. Someone else wrote it, and did it correctly.

[ossi]

I provided the your-way patch to show you how bad idea it is.

no, you didn't. you provided a terrible implementation of the idea, and keep ignoring (my) ideas how to do it better. that's a deeply dishonest way to argue a point, and your sense of entitlement to receive good will in spite of this is indeed pathetic.

[psprint]

Replying to ossi:

keep ignoring (my) ideas how to do it better

Your idea is to make the Facade incomplete, i.e. to implement only a few functions ("my point is that you don't really need a full facade - just make the simple list box a public member of the scrollable one. then accesses to the nested members still require an indirection, but no casting magic."). So you want to do it this way:

struct WListBoxScrollable {
    struct WListBox super;
    struct WScrollBox box; 
}

? If yes, then it's close to what has been implemented in the initial patch, and far from WGroup idea of andrewb.

that's a deeply dishonest way to argue a point, and your sense of entitlement to receive good will in spite of this is indeed pathetic.

Go go go! Go with your malice even more!

[ossi]

you seem really committed to "proving" that it's impossible to do what gtk and dozens of other toolkits have done.

[ossi]

i started to wonder whether re-inventing the wheel would be even necessary, and did a deep dive. turns out, yeah, kinda.

because there is an obvious match between widget toolkits and object-oriented languages, there is no shortage of c++ TUI libraries: ​FTXUI, ​FINAL CUT, ​imtui, and even the venerable ​Turbo Vision is on the table (again). and then quite some using entirely different languages.

but on the plain C side, things look grim.
i guess a honorable mention should go to ​D-Flat, which is a re-implementation of turbo vision. but given its age and state, it's hard to see it as more than a proof of concept.

in any case, there are definitely sufficiently many plain C GUI toolkits to draw inspiration from - for what you need here, the difference between GUI and TUI doesn't matter.