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

Make Multishell Scrollable #408

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@Wilma456
Contributor

Wilma456 commented Aug 8, 2017

You can now Scroll through the Programlist, if the list bigger than your Screen.

multi

@@ -115,7 +117,15 @@ local function redrawMenu()
parentTerm.setCursorPos( 1, 1 )
parentTerm.setBackgroundColor( menuOtherBgColor )
parentTerm.clearLine()
for n=1,#tProcesses do
local nCharcou = 0

This comment has been minimized.

@dan200

dan200 Sep 10, 2017

Owner

Change to "nCharCount"

@@ -8,6 +8,8 @@ local nCurrentProcess = nil
local nRunningProcess = nil
local bShowMenu = false
local bWindowsResized = false
local nScrollpos = 1

This comment has been minimized.

@dan200

dan200 Sep 10, 2017

Owner

Change to "nScrollPos"

if x == 1 and nScrollpos ~= 1 then
nScrollpos = nScrollpos - 1
redrawMenu()
elseif x == term.getSize() and bScrollRight == true then

This comment has been minimized.

@dan200

dan200 Sep 10, 2017

Owner

remove "== true"

This comment has been minimized.

@lupus590

lupus590 Sep 10, 2017

Contributor

Thinking optimisation, you may want to change the line to elseif bScrollRight and x == term.getSize() then. Doing this will mean that if bScrollRight is false then Lua will not bother calling the function to check if x is equal to the size of the terminal.

@dan200

Please make requested changes, then i'll merge this.

@Wilma456

This comment has been minimized.

Contributor

Wilma456 commented Sep 10, 2017

@dan200 Changes are now done

BombBloke and others added some commits Sep 11, 2017

@Wilma456

This comment has been minimized.

Contributor

Wilma456 commented Sep 11, 2017

I added a patch from @BombBloke which allow the scroll with the mousewhell.

nScrollPos = nScrollPos + 1
redrawMenu()
end
elseif not (bShowMenu and y == 1) then

This comment has been minimized.

@KnightMiner

KnightMiner Sep 11, 2017

I feel like this would be clearer as elseif not bShowMenu or y ~= 1 then, the and in a not is a bit more confusing to read. Arguably you could just move sEvent == "mouse_scroll" into a separate if and make this just an else statement though.

@SquidDev

This comment has been minimized.

Contributor

SquidDev commented Nov 14, 2017

Just trying this out today and noticed a issue with the scroll offset not changing when tabs are closed:

2017-11-14_23-36-42

  • Open lots of tabs in multishell (I ran bg about 10 times).
  • Scroll to the far right but keep the focus in the first tab.
  • Start closing tabs.

I think the best thing to do would be to ensure the offset is valid before each menu redraw, clamping it if required.

@Wilma456

This comment has been minimized.

Contributor

Wilma456 commented Nov 15, 2017

@SquidDev Thanks for reporting. The Bug is now fixed.

@SquidDev

This comment has been minimized.

Contributor

SquidDev commented Dec 1, 2017

Thought I'd commented this a couple of weeks ago but apparently I didn't. Sorry.

@Wilma456 The fix works, but I feel it would be nicer closing a tab scrolled the selection to the left again, so that the whole tab list is always full - rather than just having one tab on the far left.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment