From 5be28d9d5fa8f3aaa038ee922c895b0bd3663598 Mon Sep 17 00:00:00 2001 From: Max Starikov Date: Mon, 14 Sep 2026 20:51:15 +0200 Subject: [PATCH] fix code review suggestions --- .../textpad/test/EditorNavigationTest.java | 66 +++++++++++++++ .../textpad/activities/EditorActivity.java | 83 +++++++++++++++---- 2 files changed, 132 insertions(+), 17 deletions(-) diff --git a/app/src/androidTest/java/com/maxistar/textpad/test/EditorNavigationTest.java b/app/src/androidTest/java/com/maxistar/textpad/test/EditorNavigationTest.java index 3cb91e1..747896d 100644 --- a/app/src/androidTest/java/com/maxistar/textpad/test/EditorNavigationTest.java +++ b/app/src/androidTest/java/com/maxistar/textpad/test/EditorNavigationTest.java @@ -124,6 +124,39 @@ public void pointerReleaseStopsPanBeforeRemainingPointerMoves() { } } + @Test + public void finalUpAfterPanDoesNotMoveCaret() { + setEditorPreferences(true, false); + + try (ActivityScenario scenario = launchWithContent(longDocument())) { + scenario.onActivity(activity -> { + EditText editor = activity.findViewById(R.id.editText1); + resetScroll(editor, null); + editor.setSelection(0); + long downTime = SystemClock.uptimeMillis(); + + dispatch(editor, event(downTime, downTime, MotionEvent.ACTION_DOWN, + point(300, 300))); + dispatch(editor, event(downTime, downTime + 10, + pointerAction(MotionEvent.ACTION_POINTER_DOWN, 1), + point(300, 300), point(500, 500))); + dispatch(editor, event(downTime, downTime + 20, MotionEvent.ACTION_MOVE, + point(250, 250), point(450, 450))); + dispatch(editor, event(downTime, downTime + 30, + pointerAction(MotionEvent.ACTION_POINTER_UP, 1), + point(250, 250), point(450, 450))); + + int selectionStart = editor.getSelectionStart(); + int selectionEnd = editor.getSelectionEnd(); + dispatch(editor, event(downTime, downTime + 40, MotionEvent.ACTION_UP, + point(450, 450))); + + assertEquals(selectionStart, editor.getSelectionStart()); + assertEquals(selectionEnd, editor.getSelectionEnd()); + }); + } + } + @Test public void cancelAndThirdPointerTransitionsStopPan() { setEditorPreferences(true, false); @@ -201,6 +234,39 @@ public void panClampsShortAndLongContentBounds() { } } + @Test + public void horizontalPanBoundsAreCachedAndInvalidatedByTextChanges() { + setEditorPreferences(true, false); + + try (ActivityScenario scenario = launchWithContent(longDocument())) { + scenario.onActivity(activity -> { + EditText editor = activity.findViewById(R.id.editText1); + resetScroll(editor, null); + + int initialCalculations = activity.getHorizontalScrollBoundsCalculationCountForTests(); + performCompletedPan(editor, 600, 600, 800, 800, 500, 500, 700, 700); + int afterFirstPan = activity.getHorizontalScrollBoundsCalculationCountForTests(); + assertEquals(initialCalculations + 1, afterFirstPan); + + resetScroll(editor, null); + performCompletedPan(editor, 600, 600, 800, 800, 500, 500, 700, 700); + assertEquals(afterFirstPan, + activity.getHorizontalScrollBoundsCalculationCountForTests()); + + editor.setText(longDocument() + "extra text invalidates horizontal bounds"); + }); + waitForIdle(); + scenario.onActivity(activity -> { + EditText editor = activity.findViewById(R.id.editText1); + resetScroll(editor, null); + int beforeInvalidatedPan = activity.getHorizontalScrollBoundsCalculationCountForTests(); + performCompletedPan(editor, 600, 600, 800, 800, 500, 500, 700, 700); + assertEquals(beforeInvalidatedPan + 1, + activity.getHorizontalScrollBoundsCalculationCountForTests()); + }); + } + } + @Test public void goToMenuStateAndDestinationsWorkInBothLayouts() { verifyGoToNavigation(false); diff --git a/app/src/main/java/com/maxistar/textpad/activities/EditorActivity.java b/app/src/main/java/com/maxistar/textpad/activities/EditorActivity.java index 0549272..cd68198 100644 --- a/app/src/main/java/com/maxistar/textpad/activities/EditorActivity.java +++ b/app/src/main/java/com/maxistar/textpad/activities/EditorActivity.java @@ -191,6 +191,11 @@ private static final class SaveRequest { WebView mWebView; + private int cachedMaxHorizontalScroll = 0; + private int cachedHorizontalScrollEditorWidth = -1; + private boolean horizontalScrollBoundsDirty = true; + private int horizontalScrollBoundsCalculationCount = 0; + /** * Called when the activity is first created. */ @@ -218,6 +223,13 @@ public void onCreate(Bundle savedInstanceState) { mText = this.findViewById(R.id.editText1); mText.setBackgroundResource(android.R.color.transparent); mText.setOnTouchListener(new TwoFingerPanTouchListener()); + mText.addOnLayoutChangeListener((view, left, top, right, bottom, + oldLeft, oldTop, oldRight, oldBottom) -> { + if ((right - left) != (oldRight - oldLeft) + || (bottom - top) != (oldBottom - oldTop)) { + invalidateHorizontalScrollBounds(); + } + }); editTextUndoRedo = new EditTextUndoRedo(mText, this); setTextWatcher(); @@ -339,6 +351,7 @@ public void beforeTextChanged(CharSequence s, int start, int count, int after) { @Override public void onTextChanged(CharSequence s, int start, int before, int count) { + invalidateHorizontalScrollBounds(); if (suppressRecoveryTracking) { return; } @@ -760,12 +773,14 @@ private boolean isFilenameEmpty() { } void applyPreferences() { + invalidateHorizontalScrollBounds(); applyFontFace(); applyFontSize(); applyColors(); } private void disableEditorAutowrapping() { + invalidateHorizontalScrollBounds(); mText.setHorizontallyScrolling(true); mText.setHorizontalScrollBarEnabled(true); mText.setMaxLines(Integer.MAX_VALUE); @@ -1975,6 +1990,7 @@ protected void showToast(String toast_str) { private class TwoFingerPanTouchListener implements View.OnTouchListener { private boolean panningActive = false; + private boolean panGestureOwned = false; private float lastFocalX; private float lastFocalY; private int maxScrollX; @@ -1984,21 +2000,25 @@ private class TwoFingerPanTouchListener implements View.OnTouchListener { public boolean onTouch(View view, MotionEvent event) { switch (event.getActionMasked()) { case MotionEvent.ACTION_DOWN: - stopPanning(view); + endPanGesture(view); return false; case MotionEvent.ACTION_POINTER_DOWN: if (event.getPointerCount() == 2) { startPanning(view, event); } else { - return stopPanning(view); + stopActivePanning(view); } - return panningActive; + return panGestureOwned; case MotionEvent.ACTION_MOVE: - if (!panningActive) { + if (!panGestureOwned) { return false; } + if (!panningActive) { + return true; + } if (event.getPointerCount() != 2) { - return stopPanning(view); + stopActivePanning(view); + return true; } float focalX = focalX(event); float focalY = focalY(event); @@ -2012,32 +2032,42 @@ public boolean onTouch(View view, MotionEvent event) { lastFocalY = focalY; return true; case MotionEvent.ACTION_POINTER_UP: - return stopPanning(view); + if (panGestureOwned) { + stopActivePanning(view); + return true; + } + return false; case MotionEvent.ACTION_UP: case MotionEvent.ACTION_CANCEL: - return stopPanning(view); + return endPanGesture(view); default: - return panningActive; + return panGestureOwned; } } private void startPanning(View view, MotionEvent event) { panningActive = true; + panGestureOwned = true; requestParentInterception(view, true); lastFocalX = focalX(event); lastFocalY = focalY(event); Layout layout = mText.getLayout(); - maxScrollX = calculateMaxHorizontalScroll(layout); + maxScrollX = getMaxHorizontalScroll(layout); maxScrollY = calculateMaxVerticalScroll(layout); } - private boolean stopPanning(View view) { - boolean wasPanning = panningActive; - panningActive = false; - if (wasPanning) { + private void stopActivePanning(View view) { + if (panningActive) { requestParentInterception(view, false); } - return wasPanning; + panningActive = false; + } + + private boolean endPanGesture(View view) { + boolean shouldConsume = panGestureOwned || panningActive; + stopActivePanning(view); + panGestureOwned = false; + return shouldConsume; } private void requestParentInterception(View view, boolean disallow) { @@ -2079,17 +2109,36 @@ private void panContent(int deltaX, int deltaY, int maxScrollX, int maxScrollY) mText.scrollTo(newX, mText.getScrollY()); } - private int calculateMaxHorizontalScroll(Layout layout) { - if (layout == null) { + private int getMaxHorizontalScroll(Layout layout) { + int editorWidth = mText.getWidth(); + if (layout == null || editorWidth <= 0) { return 0; } + if (horizontalScrollBoundsDirty || cachedHorizontalScrollEditorWidth != editorWidth) { + cachedMaxHorizontalScroll = calculateMaxHorizontalScroll(layout, editorWidth); + cachedHorizontalScrollEditorWidth = editorWidth; + horizontalScrollBoundsDirty = false; + } + return cachedMaxHorizontalScroll; + } + + private void invalidateHorizontalScrollBounds() { + horizontalScrollBoundsDirty = true; + } + + public int getHorizontalScrollBoundsCalculationCountForTests() { + return horizontalScrollBoundsCalculationCount; + } + + private int calculateMaxHorizontalScroll(Layout layout, int editorWidth) { + horizontalScrollBoundsCalculationCount++; float maxLineWidth = 0; for (int line = 0; line < layout.getLineCount(); line++) { maxLineWidth = Math.max(maxLineWidth, layout.getLineWidth(line)); } int contentWidth = (int) Math.ceil(maxLineWidth) + mText.getTotalPaddingLeft() + mText.getTotalPaddingRight(); - return Math.max(0, contentWidth - mText.getWidth()); + return Math.max(0, contentWidth - editorWidth); } private int calculateMaxVerticalScroll(Layout layout) {