== 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?
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256>
Midnight Commander <https://midnight-commander.org>
Midnight Development Center
Comment (by 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...
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:1>
Comment (by andrew_b):
Replying to [ticket:4256 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).
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:2>
Comment (by kybl):
Replying to [comment:1 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:3>
Comment (by kybl):
Replying to [comment:2 andrew_b]:
> Replying to [ticket:4256 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 :-)
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:4>
Comment (by andrew_b):
Replying to [comment:4 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:5>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:6>
Comment (by 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?
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:7>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:8>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:9>
Comment (by 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 [https
://midnight-commander.org/attachment/ticket/4256/another-types.png
anothertypes.png].
See patch and screenshot. What do you think?
[comment:5 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?
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:10>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:11>
Comment (by 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 [comment:10 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.
> [comment:5 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:12>
Comment (by 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-based implementation be dropped? 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:13>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:14>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:15>
Comment (by psprint):
Could someone answer?
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:16>
Comment (by 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?
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:17>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:18>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:19>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:20>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:21>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:22>
Comment (by psprint):
Replying to [comment:22 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?
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:23>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:24>
Comment (by 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...
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:26>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:27>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:28>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:29>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:31>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:32>
Comment (by 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.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:33>
Comment (by psprint):
Replying to [comment:33 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!
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:34>
Comment (by ossi):
you seem really committed to "proving" that it's impossible to do what gtk
and dozens of other toolkits have done.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4256#comment:35>
Comment (by 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:
[https://github.com/ArthurSonzogni/FTXUI FTXUI],
[https://github.com/gansm/finalcut FINAL CUT],
[https://github.com/ggerganov/imtui imtui], and even the venerable
[