Horizontal scrollbars for all consoles + scrollbar fixes by Edru2 · Pull Request #4085 · Mudlet/Mudlet

Edru2 · 2020-09-25T17:32:04Z

Brief overview of PR changes/additions

This PR adds new functions to enable/disable horizontal scrollbars for all user consoles (mainconsole/miniconsole/userwindow)
new functions:

enableHorizontalScrollBar([windowName])
disableHorizontalScrollBar([windowName])

Note that horizontal scrollbars disappear(hide) if the window is big enough to fit all characters.

This PR also adds the possibility to scroll horizontal with a horizontal mouse-wheel (testing needed) and fixes 2 long time issues with scrollbars.

adds also the possibility to hide main window vertical scrollbars

Motivation for adding to Mudlet

discussion on discord
fix #612
fix #1702
fix #3054

Other info (issues closed, discussion etc)

The mouse wheel code was (mostly) provided by @SlySven

add-deployment-links · 2020-09-25T17:32:42Z

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

vadi2

flatbox = Geyser.MiniConsole:new({
  name="flatbox",
  x="70%", y="50%",
  wrapAt = 10,
  width="50c", height="2c",
})
flatbox:setColor("red")
flatbox:echo("ABCDEFGHIJ") -- that is 10 letters
flatbox:enableHorizontalScrollBar()

Scrolling with the mouse here is pretty difficult, because it seems to be all or nothing. Examining the docs, it looks like the step needs to be adjusted dynamically?

Edru2 · 2020-09-25T18:05:29Z

flatbox = Geyser.MiniConsole:new({
name="flatbox",
x="70%", y="50%",
wrapAt = 10,
width="50c", height="2c",
})
flatbox:setColor("red")
flatbox:echo("ABCDEFGHIJ") -- that is 10 letters
flatbox:enableHorizontalScrollBar()

hmm there is also nothing to scroll as you only echo 10 characters but the width is 50 and it wraps at 10.
If I copy your code I don't have a scrollbar at all.

See first post
Note that horizontal scrollbars disappear(hide) if the window is big enough to fit all characters.

vadi2 · 2020-09-25T18:07:21Z

Does it re-check if it should hide? I created one where a scrollbar was needed then widened the miniconsole in testing, now I have:

Edru2 · 2020-09-25T18:18:15Z

Does it re-check if it should hide? I created one where a scrollbar was needed then widened the miniconsole in testing, now I have:

Okay I noticed there is an issue if there is only one line and the scrollbar already visible.
It should work if you echo more lines.

Will check what is causing it and how to fix it.
Probably I shouldn't make the scrollbar visible before it is needed.

vadi2 · 2020-09-25T18:21:05Z

When :clear() is called, the scrollbar should hide, right?

Edru2 · 2020-09-25T18:23:49Z

When :clear() is called, the scrollbar should hide, right?

No as it doesn't reset mScreenOffset yet 🤦‍♂️
(will add that)

Does it re-check if it should hide? I created one where a scrollbar was needed then widened the miniconsole in testing, now I have:

will convert to draft for now (is less ready then I thought)

Thanks for testing

vadi2 · 2020-09-26T10:30:24Z

src/TConsole.cpp


+    int scrollBarHeight = 0;
+    if (mpHScrollBar && mpHScrollBar->isVisible()) {
+        scrollBarHeight = 15;


This might not be safe - a scrollbar could be styled by the OS to be a different size. Is there a way to get the real value?

We set the same size for the vertical scrollbar BTW - and if you do not specify a size it can grow to fill the layout it is in - when I was playing around and trying to clean up the messy layout done manually in the TConsole constructor (to restructure things to):

Grid Layout: | | Col:| 0 | 1 | | Row:| | --- +--------------------+ |+---------------++-+| || ||V|| || || || || Upper ||S|| 0 || Pane ||-|| || ||B|| || ||a|| || ||r|| |+---------------++-+| --- |+---------------+XXX| 1 || H S-bar |XXX| |+---------------+XXX| --- |+------------------+| || || || Lower || 2 || Pane || || || |+------------------+| +--------------------+ =================== Note: the command line stuff (and buttons and Latency/Time) are in a separate layout in a parent of this widget

In the absence of more explicit settings column 1 and row 1 will adopt the same width as column 0 and row 0 (and 2 but that is regulated)...

Edru2 · 2020-09-26T13:25:05Z

The issues should be fixed but the example your provide would still not work as the scrollbar is (or should at least) 15px big and that won't fit in your miniconsole.
I provide a slightly adjusted example ( one line of text and enough space for the scrollbar)

flatbox = flatbox or Geyser.MiniConsole:new({
  name="flatbox",
  x="70%", y="50%",
  wrapAt = 10,
  width="20c"
})

local scrollBarHeight = 15
local w, h = calcFontSize(flatbox.fontSize)

flatbox:enableHorizontalScrollBar()
flatbox:resize(nil, h + scrollBarHeight)
flatbox:setColor("red")
flatbox:echo("ABCDEFGHIJKLMNOPQRSTUVWXYZ") -- that is 26 letters

make horizontal scrollbars part of the main console. which means they get also resizes if borders resize. This looks much better then before and the code for the resize event is not needed anymore.

and also if it hides due to control+return

fix Mudlet#3054 and alll issues described in it

forgot one

Kebap · 2020-10-04T18:38:03Z

Reviewing issue #3054 this version is still acting a bit strange (but much improved to before)

Not sure how exhausting this work seems to you, but for me these would seem nice to have:

For example, when scrolling with keyboard/mouse-wheel/drag-and-dropping-via-mouse, the scrollbar will reduce its size and stay smaller until moved back down to the end. Whereas before it didn't change and actually move at all.
For example, the lower pane will "jump" a small fraction of a line height and stay that way, until scrolling ended once again. I am not aware if this was happening before at all.
For example, the upper pane will still show parts of text which are actually visible in lower pane already, therefore doubling some text lines unneccessarily. This seems unchanged to before, so this part of Issues with the scrolling handler #3054 may need to be reissued anew after this PR closes that main issue.
For example, if scrolling up one mouse-wheel bit, then scrolling down immediately again after, the scroll-bar will act unexpectedly: a) up - it will shrink and stay at bottom, b) down - it will stay shrunk and actually move up, c) down again - only now it will go down to the end and grow back to starting size.
Same is happening if scrolling with page-up and page-down keys, only to an even larger extent: a) page up - panes will split, upper pane has scrolled up, but scroll-bar stays at bottom-most position, but has shrunk in size, b) page down - upper pane will scroll contents down, actually to a perfect fit with lower pane contents, but due to 3) above, the scroll bar is not back at bottom-most position, instead moved up a bit compared to the last key-press, c) page down - upper pane now shows all contents of lower pane as well, but still scrolling has not ended, scroll-bar has not reached bottom-most position, yet. d) page down - only now scrolling ends, panes have been combined into one, scroll-bar is back at original size and position.
So in total 4 keypresses: up, down, down, down, to return back to the original state. This just seems confusing, even without the scroll-bar size changes and unexpected moving around.

See some of this in action for your comparison:

Edru2 · 2020-10-04T19:01:15Z

Thanks for testing!

This is not fixable because the scrollbar gets its size from the upperPane which changes in size as soon as a split happens.
This also happens in 4.9.1 but wasn't as obvious as the split didn't happen immediately and also the scrollbar didn't change size when using mousewheel/pageup/pagedown probably still able to fix it

This wouldn't happen anymore if we apply SlySvens layout suggestion but then the whole scrollbar would "jump" as soon as
the split happens because the scrollbar would be attached to the upperPane (something to discuss about)

Additionally this is almost not noticeable anymore once there are more lines on the screen.

I tested it and it is the same behavior as in 4.9.1 (maybe something for another pr)
this is also the same in 4.9.1 and I'm not sure how to fix (need to look at it again)

4 and 5 are problematic and shouldn't happen. (definitely need to look at it again as I thought I fixed that already)

fix changing size and got rid of bufferscrollup as it wasn't needed and didn't do what it was supposed to anyway

horizontal scroll by highlighting the same way vertical scroll does

scrolling for horizontal seem to be to fast if the same values as for vertical are used. horizontal position gets resetet if new lines appear the same way it happens for vertical scrollbar (it doesn't happen for if the split is active)

iLPdev

works gorgeously for me in win10pro v1909 build 18363.1082:

pointer hover designates container to scroll.
scrollbar resizes as buffer fills.
buffer display is appropriately split and scroll at designated main window boundary.

Muuuah!! Thanks!

atari2600tim · 2020-10-06T02:55:39Z

src/TTextEdit.cpp

+    if (deltaY < 0) {
+        mpConsole->scrollDown(k);
+        handled = true;
+    } else if (e->delta() > 0) {


Scrolling left and right works for me, but scrolling to the left causes it to also scroll up at the same time. I think that this should be deltaY instead of e->delta().

Thanks for spotting that. Please retest :)

e->delta() should be deltaY

atari2600tim · 2020-10-06T13:37:11Z

Scrolls properly now with wheel or bar and no problem doing one then the other, and I was able to enable and disable horizontal scroll bar through lua. My simple UI doesn't have the separate windows and all that, but what I did try, main window all works good.

vadi2

Looks fine, I just see a splitter appear for a split moment while the profile is loading and that's new.

Thanks a lot for the work in this! It almost feels a bit unusual to have the scrollbar... working! 😀

vadi2 · 2020-10-06T13:56:47Z

src/TLuaInterpreter.cpp

 {
    int n = lua_gettop(L);
-    QString windowName;
+    QString windowName = "main";


Suggested change

QString windowName = "main";

QString windowName = QStringLiteral("main");

vadi2 · 2020-10-06T13:56:57Z

src/TLuaInterpreter.cpp

 {
    int n = lua_gettop(L);
-    QString windowName;
+    QString windowName = "main";


Suggested change

QString windowName = "main";

QString windowName = QStringLiteral("main");

vadi2 · 2020-10-06T13:57:21Z

src/TLuaInterpreter.cpp

+int TLuaInterpreter::disableHorizontalScrollBar(lua_State* L)
+{
+    int n = lua_gettop(L);
+    QString windowName = "main";


Suggested change

QString windowName = "main";

QString windowName = QStringLiteral("main");

vadi2 · 2020-10-06T13:59:15Z

src/TTextEdit.cpp

        mFontHeight = mFontAscent + mFontDescent;
    }
    mScreenHeight = visibleRegion().boundingRect().height() / mFontHeight;
+    if (!mIsLowerPane){


Suggested change

if (!mIsLowerPane){

if (!mIsLowerPane) {

atari2600tim · 2020-10-06T16:30:03Z

This will almost close #498 too, except there is still QWheelEvent->delta() in https://github.com/Mudlet/Mudlet/blob/development/src/glwidget.cpp#L2254

@atari2600tim

* fix scrollbar and allow horizontal wheel * fix for characters didn't get cleared * enable/disable horizontal scrollbar * fix issues and resize console if horizontal scrollbar show/hide * get scrollbarheight dynamically * change layout design make horizontal scrollbars part of the main console. which means they get also resizes if borders resize. This looks much better then before and the code for the resize event is not needed anymore. * remove unneeded console resize * update scrollbar when lowerPane hides due to scrolling and also if it hides due to control+return * add horizontal scrollbar to geyser miniconsole by cons * fix issues Mudlet#3054 fix Mudlet#3054 and alll issues described in it * Update TCommandLine.cpp forgot one * fix changing size fix changing size and got rid of bufferscrollup as it wasn't needed and didn't do what it was supposed to anyway * get rid of forgotten debugging messages * scroll horizontal by highlighting text horizontal scroll by highlighting the same way vertical scroll does * horizontal scrolling was to fast + reset position scrolling for horizontal seem to be to fast if the same values as for vertical are used. horizontal position gets resetet if new lines appear the same way it happens for vertical scrollbar (it doesn't happen for if the split is active) * set mMaxHRange to 0 if to small * copy/paste fail spottet by @atari2600tim e->delta() should be deltaY * suggestions

Edru2 added 3 commits September 25, 2020 12:30

fix scrollbar and allow horizontal wheel

c64d9e1

fix for characters didn't get cleared

b9a7b9d

enable/disable horizontal scrollbar

1a1372c

Edru2 requested a review from a team as a code owner September 25, 2020 17:32

Edru2 requested review from a team September 25, 2020 17:32

vadi2 reviewed Sep 25, 2020

View reviewed changes

Edru2 marked this pull request as draft September 25, 2020 18:24

fix issues and resize console if horizontal scrollbar show/hide

61fab8d

vadi2 reviewed Sep 26, 2020

View reviewed changes

vadi2 assigned Edru2 Sep 26, 2020

get scrollbarheight dynamically

f453003

Edru2 added 2 commits September 26, 2020 20:18

change layout design

8e55c93

make horizontal scrollbars part of the main console. which means they get also resizes if borders resize. This looks much better then before and the code for the resize event is not needed anymore.

remove unneeded console resize

aefeedc

Edru2 marked this pull request as ready for review September 27, 2020 06:45

vadi2 changed the title ~~enable/disable horizontal scrollbars for all consoles~~ Enable/disable horizontal scrollbars for all consoles Sep 27, 2020

Edru2 and others added 6 commits September 27, 2020 10:26

update scrollbar when lowerPane hides due to scrolling

ff38bf6

and also if it hides due to control+return

add horizontal scrollbar to geyser miniconsole by cons

724cb41

fix issues Mudlet#3054

296d0ae

fix Mudlet#3054 and alll issues described in it

Merge branch 'development' into fix-scrollbars

784158d

Update TCommandLine.cpp

19cb8a3

forgot one

Merge branch 'development' into fix-scrollbars

53e6fe2

Edru2 added 2 commits October 4, 2020 21:09

Merge branch 'development' into fix-scrollbars

d8fac1f

fix changing size

4b0c4fb

fix changing size and got rid of bufferscrollup as it wasn't needed and didn't do what it was supposed to anyway

vadi2 changed the title ~~Enable/disable horizontal scrollbars for all consoles~~ Horizontal scrollbars for all consoles + scrollbar fixes Oct 5, 2020

Edru2 added 4 commits October 5, 2020 11:21

get rid of forgotten debugging messages

91458fb

scroll horizontal by highlighting text

b12be75

horizontal scroll by highlighting the same way vertical scroll does

horizontal scrolling was to fast + reset position

42fd504

scrolling for horizontal seem to be to fast if the same values as for vertical are used. horizontal position gets resetet if new lines appear the same way it happens for vertical scrollbar (it doesn't happen for if the split is active)

set mMaxHRange to 0 if to small

f6f242f

iLPdev reviewed Oct 5, 2020

View reviewed changes

iLPdev mentioned this pull request Oct 5, 2020

Issues with the scrolling handler #3054

Closed

atari2600tim reviewed Oct 6, 2020

View reviewed changes

copy/paste fail spottet by @atari2600tim

48215c4

e->delta() should be deltaY

vadi2 approved these changes Oct 6, 2020

View reviewed changes

suggestions

efe3b7a

vadi2 merged commit 587f962 into Mudlet:development Oct 7, 2020

Edru2 mentioned this pull request Oct 30, 2020

revert TText wheelEvent (try to fix macOs scroll issue) #4229

Merged

atari2600tim mentioned this pull request Dec 13, 2020

Enhance: make TConsole scrolling smoother #4445

Merged

	QString windowName = "main";
	QString windowName = QStringLiteral("main");

Uh oh!

Conversation

Edru2 commented Sep 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Brief overview of PR changes/additions

Motivation for adding to Mudlet

Other info (issues closed, discussion etc)

Uh oh!

add-deployment-links bot commented Sep 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test versions

Uh oh!

vadi2 left a comment

Choose a reason for hiding this comment

Uh oh!

Edru2 commented Sep 25, 2020

Uh oh!

vadi2 commented Sep 25, 2020

Uh oh!

Edru2 commented Sep 25, 2020

Uh oh!

vadi2 commented Sep 25, 2020

Uh oh!

Edru2 commented Sep 25, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Edru2 commented Sep 26, 2020

Uh oh!

Kebap commented Oct 4, 2020

Uh oh!

Edru2 commented Oct 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iLPdev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atari2600tim commented Oct 6, 2020

Uh oh!

vadi2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atari2600tim commented Oct 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Edru2 commented Sep 25, 2020 •

edited

Loading

add-deployment-links bot commented Sep 25, 2020 •

edited

Loading

Edru2 commented Oct 4, 2020 •

edited

Loading

iLPdev left a comment •

edited

Loading