Skip to content

Commit

Permalink
add tests for #9955.b as well
Browse files Browse the repository at this point in the history
  • Loading branch information
@zadjii-msft
zadjii-msft committed Apr 28, 2021
1 parent 33d29fa commit 0fefc5b
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 7 deletions.
8 changes: 4 additions & 4 deletions src/cascadia/TerminalControl/ControlInteractivity.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

// Update the Core's viewport position, and raise a
// ScrollPositionChanged event to update the scrollbar
_updateScrollbar(newValue);
UpdateScrollbar(newValue);

// Use this point as our new scroll anchor.
_touchAnchor = newTouchPoint;
Expand Down Expand Up @@ -459,7 +459,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

// Update the Core's viewport position, and raise a
// ScrollPositionChanged event to update the scrollbar
_updateScrollbar(newValue);
UpdateScrollbar(newValue);

if (isLeftButtonPressed)
{
Expand All @@ -485,12 +485,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - newValue: The new top of the viewport
// Return Value:
// - <none>
void ControlInteractivity::_updateScrollbar(const double newValue)
void ControlInteractivity::UpdateScrollbar(const double newValue)
{
// Set this as the new value of our internal scrollbar representation.
// We're doing this so we can accumulate fractional amounts of a row to
// scroll each time the mouse scrolls.
_internalScrollbarPosition = newValue;
_internalScrollbarPosition = std::clamp<double>(newValue, 0.0, _core->BufferHeight());

// If the new scrollbar position, rounded to an int, is at a different
// row, then actually update the scroll position in the core, and raise
Expand Down
4 changes: 3 additions & 1 deletion src/cascadia/TerminalControl/ControlInteractivity.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const int32_t delta,
const til::point pixelPosition,
const ::Microsoft::Console::VirtualTerminal::TerminalInput::MouseButtonState state);

void UpdateScrollbar(const double newValue);

#pragma endregion

bool CopySelectionToClipboard(bool singleLine,
Expand Down Expand Up @@ -124,7 +127,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
bool _canSendVTMouseInput(const ::Microsoft::Terminal::Core::ControlKeyStates modifiers);

void _sendPastedTextToConnection(std::wstring_view wstr);
void _updateScrollbar(const double newValue);
til::point _getTerminalPosition(const til::point& pixelPosition);

void _coreScrollPositionChanged(const IInspectable& /*sender*/, const Control::ScrollPositionChangedArgs& args);
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalControl/TermControl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1302,8 +1302,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return;
}

const auto newValue = static_cast<int>(args.NewValue());
_core->UserScrollViewport(newValue);
const auto newValue = args.NewValue();
_interactivity->UpdateScrollbar(newValue);

// User input takes priority over terminal events so cancel
// any pending scroll bar update if the user scrolls.
Expand Down
110 changes: 110 additions & 0 deletions src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ namespace ControlUnitTests

TEST_METHOD(CreateSubsequentSelectionWithDragging);
TEST_METHOD(ScrollWithSelection);
TEST_METHOD(TestScrollWithTrackpad);
TEST_METHOD(TestQuickDragOnSelect);

TEST_CLASS_SETUP(ClassSetup)
Expand Down Expand Up @@ -228,11 +229,13 @@ namespace ControlUnitTests
Log::Comment(L"Scroll down 21 more times, to the bottom");
for (int i = 0; i < 21; ++i)
{
Log::Comment(NoThrowString().Format(L"---scroll down #%d---", i));
expectedTop++;
interactivity->MouseWheel(modifiers,
-WHEEL_DELTA,
til::point{ 0, 0 },
{ false, false, false });
Log::Comment(NoThrowString().Format(L"internal scrollbar pos:%f", interactivity->_internalScrollbarPosition));
}
Log::Comment(L"Scrolling up more should do nothing");
expectedTop = 21;
Expand Down Expand Up @@ -410,6 +413,113 @@ namespace ControlUnitTests
VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionEnd());
}

void ControlInteractivityTests::TestScrollWithTrackpad()
{
WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{};

auto [settings, conn] = _createSettingsAndConnection();
auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn);
_standardInit(core, interactivity);
// For the sake of this test, scroll one line at a time
interactivity->_rowsToScroll = 1;

// int expectedTop = 0;
// int expectedViewHeight = 20;
// int expectedBufferHeight = 20;

// auto scrollChangedHandler = [&](auto&&, const Control::ScrollPositionChangedArgs& args) mutable {
// VERIFY_ARE_EQUAL(expectedTop, args.ViewTop());
// VERIFY_ARE_EQUAL(expectedViewHeight, args.ViewHeight());
// VERIFY_ARE_EQUAL(expectedBufferHeight, args.BufferSize());
// };
// core->ScrollPositionChanged(scrollChangedHandler);
// interactivity->ScrollPositionChanged(scrollChangedHandler);

for (int i = 0; i < 40; ++i)
{
// Log::Comment(NoThrowString().Format(L"Writing line #%d", i));
// The \r\n in the 19th loop will cause the view to start moving
// if (i >= 19)
// {
// expectedTop++;
// expectedBufferHeight++;
// }

conn->WriteInput(L"Foo\r\n");
}
// We printed that 40 times, but the final \r\n bumped the view down one MORE row.
VERIFY_ARE_EQUAL(20, core->_terminal->GetViewport().Height());
VERIFY_ARE_EQUAL(21, core->ScrollOffset());
VERIFY_ARE_EQUAL(20, core->ViewHeight());
VERIFY_ARE_EQUAL(41, core->BufferHeight());

Log::Comment(L"Scroll up a line");
const auto modifiers = ControlKeyStates();
// expectedBufferHeight = 41;
// expectedTop = 20;

// Deltas that I saw while scrolling with the surface laptop trackpad
// were on the range [-22, 7], though I'm sure they could be greater in
// magnitude.
//
// WHEEL_DELTA is 120, so we'll use 24 for now as the delta, just so the tests don't take forever.

const int delta = WHEEL_DELTA / 5;
const til::point mousePos{ 0, 0 };
TerminalInput::MouseButtonState state{ false, false, false };

interactivity->MouseWheel(modifiers, delta, mousePos, state); // 1/5
VERIFY_ARE_EQUAL(21, core->ScrollOffset());

Log::Comment(L"Scroll up 4 more times. Once we're at 3/5 scrolls, "
L"we'll round the internal scrollbar position to scrolling to the next row.");
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 2/5
VERIFY_ARE_EQUAL(21, core->ScrollOffset());
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 3/5
VERIFY_ARE_EQUAL(20, core->ScrollOffset());
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 4/5
VERIFY_ARE_EQUAL(20, core->ScrollOffset());
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 5/5
VERIFY_ARE_EQUAL(20, core->ScrollOffset());

Log::Comment(L"Jump to line 5, so we can scroll down from there.");
interactivity->UpdateScrollbar(5);
VERIFY_ARE_EQUAL(5, core->ScrollOffset());
Log::Comment(L"Scroll down 5 times, at which point we should accumulate a whole row of delta.");
interactivity->MouseWheel(modifiers, -delta, mousePos, state); // 1/5
VERIFY_ARE_EQUAL(5, core->ScrollOffset());
interactivity->MouseWheel(modifiers, -delta, mousePos, state); // 2/5
VERIFY_ARE_EQUAL(5, core->ScrollOffset());
interactivity->MouseWheel(modifiers, -delta, mousePos, state); // 3/5
VERIFY_ARE_EQUAL(6, core->ScrollOffset());
interactivity->MouseWheel(modifiers, -delta, mousePos, state); // 4/5
VERIFY_ARE_EQUAL(6, core->ScrollOffset());
interactivity->MouseWheel(modifiers, -delta, mousePos, state); // 5/5
VERIFY_ARE_EQUAL(6, core->ScrollOffset());

Log::Comment(L"Jump to the bottom.");
interactivity->UpdateScrollbar(21);
VERIFY_ARE_EQUAL(21, core->ScrollOffset());
Log::Comment(L"Scroll a bit, then emit a line of text. We should reset our internal scroll position.");
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 1/5
VERIFY_ARE_EQUAL(21, core->ScrollOffset());
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 2/5
VERIFY_ARE_EQUAL(21, core->ScrollOffset());

conn->WriteInput(L"Foo\r\n");
VERIFY_ARE_EQUAL(22, core->ScrollOffset());
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 1/5
VERIFY_ARE_EQUAL(22, core->ScrollOffset());
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 2/5
VERIFY_ARE_EQUAL(22, core->ScrollOffset());
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 3/5
VERIFY_ARE_EQUAL(21, core->ScrollOffset());
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 4/5
VERIFY_ARE_EQUAL(21, core->ScrollOffset());
interactivity->MouseWheel(modifiers, delta, mousePos, state); // 5/5
VERIFY_ARE_EQUAL(21, core->ScrollOffset());
}

void ControlInteractivityTests::TestQuickDragOnSelect()
{
// This is a test for GH#9955.c
Expand Down

1 comment on commit 0fefc5b

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misspellings found, please review:

  • draging
  • infor
  • temrinal
To accept these changes, run the following commands from this repository on this branch 
pushd $(git rev-parse --show-toplevel)
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"aef aspnet boostorg BSODs Cac COINIT dahall DEFCON developercommunity fde fea fmtlib hotkeys HPCON isocpp llvm mintty msvcrtd Nc NVDA pinam QOL remoting UNINIT Unk unte vcrt what3words xamarin "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect/0fefc5b9eacf44ebfb7cbecd69f665e927c45d15.txt";
use File::Path qw(make_path);
make_path ".github/actions/spelling/expect";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"cac coinit draging hpcon infor LLVM MSVCRTD nc Remoting temrinal uninit unk "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
popd
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

🗜️ If you see a bunch of garbage and it relates to a binary-ish string, please add a file path to the .github/actions/spelling/excludes.txt file instead of just accepting the garbage.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

Please sign in to comment.