-
-
Notifications
You must be signed in to change notification settings - Fork 247
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
Horizontal scrollbars for all consoles + scrollbar fixes #4085
Conversation
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
hmm there is also nothing to scroll as you only echo 10 characters but the width is 50 and it wraps at 10. See first post |
Okay I noticed there is an issue if there is only one line and the scrollbar already visible. Will check what is causing it and how to fix it. |
When |
src/TConsole.cpp
Outdated
@@ -673,19 +679,23 @@ void TConsole::resizeEvent(QResizeEvent* event) | |||
int x = event->size().width(); | |||
int y = event->size().height(); | |||
|
|||
int scrollBarHeight = 0; | |||
if (mpHScrollBar && mpHScrollBar->isVisible()) { | |||
scrollBarHeight = 15; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)...
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. 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
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:
See some of this in action for your comparison: |
Thanks for testing!
This wouldn't happen anymore if we apply SlySvens layout suggestion but then the whole scrollbar would "jump" as soon as Additionally this is almost not noticeable anymore once there are more lines on the screen.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!
src/TTextEdit.cpp
Outdated
if (deltaY < 0) { | ||
mpConsole->scrollDown(k); | ||
handled = true; | ||
} else if (e->delta() > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting that. Please retest :)
e->delta() should be deltaY
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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! 😀
src/TLuaInterpreter.cpp
Outdated
@@ -2659,7 +2659,7 @@ int TLuaInterpreter::setConsoleBufferSize(lua_State* L) | |||
int TLuaInterpreter::enableScrollBar(lua_State* L) | |||
{ | |||
int n = lua_gettop(L); | |||
QString windowName; | |||
QString windowName = "main"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QString windowName = "main"; | |
QString windowName = QStringLiteral("main"); |
src/TLuaInterpreter.cpp
Outdated
@@ -2680,7 +2680,7 @@ int TLuaInterpreter::enableScrollBar(lua_State* L) | |||
int TLuaInterpreter::disableScrollBar(lua_State* L) | |||
{ | |||
int n = lua_gettop(L); | |||
QString windowName; | |||
QString windowName = "main"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QString windowName = "main"; | |
QString windowName = QStringLiteral("main"); |
src/TLuaInterpreter.cpp
Outdated
int TLuaInterpreter::disableHorizontalScrollBar(lua_State* L) | ||
{ | ||
int n = lua_gettop(L); | ||
QString windowName = "main"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QString windowName = "main"; | |
QString windowName = QStringLiteral("main"); |
src/TTextEdit.cpp
Outdated
@@ -267,6 +287,9 @@ void TTextEdit::updateScreenView() | |||
mFontHeight = mFontAscent + mFontDescent; | |||
} | |||
mScreenHeight = visibleRegion().boundingRect().height() / mFontHeight; | |||
if (!mIsLowerPane){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!mIsLowerPane){ | |
if (!mIsLowerPane) { |
This will almost close #498 too, except there is still QWheelEvent->delta() in https://github.com/Mudlet/Mudlet/blob/development/src/glwidget.cpp#L2254 |
* 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
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:
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