I think I'm least likely to break something if I do this as explicitly
as possible rather than trying to figure out the right helper to put it
in once and for all. So just add some boilerplate to reset all state at
the start of each onpress1 handler. This seems like a good choice since
we aren't planning to add a lot more menus that would quadratically
explode the boilerplate.
Scenario I was seeing:
* on an Android phone
* when font size is small
* if I scrolled down a bit
* and then tapped on the editor
Then before this commit, the scrollbar would move. Even though I was
tapping nowhere near the scrollbar. Or even the settings area!
I carefully bisected this down to commit 7ca2006145 and this code in
on.mouse_release:
if on_area(Settings_menu_area, x,y) then
return
end
What was actually going on in the above scenario:
* on a device with a small screen
* after I open the settings menu once
* the scrollbar overlaps the erstwhile setting menu, so that dragging
the scrollbar doesn't trigger on.mouse_receive
* as a result the scrollbar is still active when I tap the editor.
Holy smokes!
We'll end up calling Text.redraw_all anyway, which will clear starty and
much more besides.
We'll still conservatively continue clearing starty in a few places
where there's a possibility that Text.redraw_all may not be called. This
change is riskier than most.
scenario 1: tap on scrollbar area off scrollbar.
With the previous commit the app would crash.
scenario 2: drag down a bit, then let go, then drag the scrollbar again.
With the previous commit the scrollbar would start again from 0.
Root cause: units mismatch (pixels vs normalized y between 0 and 1)
scenario: just tap somewhere on a scrollbar.
There should be no scrolling. But before this commit there would be.
This made the scrollbars feel unstable.
I finally figured this out while noodling over a follow-up question to
the previous commit:
Why was dragging the scrollbar down ever leaving a trail of more than
just the top line's `starty`? In other words, why was my example:
line 1: 30
line 2: 60
line 3: 90
line 4: 30
line 5: 30
line 6: 30
...
..and not:
line 1: 30
line 2: 30
line 3: 30
line 4: 30
line 5: 30
line 6: 30
...
??
The answer: when I grab a scrollbar it always used to jump down!
Usability issues can either exacerbate bugs or make them harder to
diagnose. If I'd implemented scrollbars like this from the start, we'd
either never have noticed the problem of the previous commit or fixed it
much more quickly.
scenario:
drag the editor scrollbar down more than a page
tap near the bottom of the editor. Tapping works
tap near the top of the editor. Nothing happens.
The cause: as you drag the scrollbar, starty gets set for lines on
screen. However, the lines going above the screen don't get their starty
reset.
Why even would any tapping work? Answer: dragging the scrollbar is
always contiguous, never random access. So when you drag a little bit,
the top line gets its starty set. When you drag a little more the next
line has its starty set. As a result, the usual pattern with the bug
was something like:
line 1: 30
line 2: 60
line 3: 90
line 4: 30
line 5: 30
line 6: 30
line 7: 30
line 8: 30
...
As a result, low `y`s (<90 in this made-up example) get caught in those
first few lines, but high `y`s get to the right line below.
Root cause: any time you scroll, you have to call Text.redraw_all. That
will cause `starty`s to look like this:
line 1: nil
line 2: nil
line 3: nil
line 4: nil
line 5: nil
line 6: 30 <== screen_top1
line 7: 60
...
Just checking mouse.isDown works if the editor is the entirety of the
app, as is true in this fork. However, we often want to introduce other
widgets. We'd like tapping on them to not cause the selection to flash:
https://news.ycombinator.com/context?id=38404923&submission=38397715
The right architecture to enforce this is: have each layer of the UI
maintain its own state machine between mouse_press and mouse_release
events. And only check the state machine in the next level down rather
than lower layers or the bottommost layer of raw LÖVE.
This change needs to get backported to all the other forks. We want to
retain the ability to draw other UI elements in our apps, and that means
we can't always assume that a mouse press and mouse release will happen
together. Instead, each layer should manage its own state machine, and
only check for events from the immediately lower layer.
This idea generalizes commit c3cdbb52a.
Problem: on mobile devices, tapping on the editor causes the selection
to flash for a second. It looks like there's a longer time between mouse
press and release events.
Scenarios tested with this change:
* click on the editor to move the cursor
* drag-select using the mouse
* shift-select using keyboard
* shift-click to select
The active state has 2 uses:
* It gives some visual feedback that a button has been pressed.
Otherwise, tapping a button gave no feedback that the tap had
occurred. Particularly on a phone screen where fat-fingering is
easier, hitting the 'save' button in particular gave no hint that the
file had actually been saved. Which causes some anxiety.
* It suppresses all mouse taps during the activation time. For example,
this helps keep tapping say an overflow button from selecting text in
the editor.
Before this commit we were only saving settings when explicitly hitting
the 'settings' button to dismiss the menu. But clicking anywhere outside
the menu dismisses it.
Kartik K. Agaram