Skip to content

fix code review suggestions #240

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.

Sign up for GitHub

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

Merged
maxistar merged 1 commit into from
Sep 15, 2026
Merged

fix code review suggestions #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions app/src/androidTest/java/com/maxistar/textpad/test/EditorNavigationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,39 @@ public void pointerReleaseStopsPanBeforeRemainingPointerMoves() {
}
}

@Test
public void finalUpAfterPanDoesNotMoveCaret() {
setEditorPreferences(true, false);

try (ActivityScenario<EditorActivity> 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);
Expand Down Expand Up @@ -201,6 +234,39 @@ public void panClampsShortAndLongContentBounds() {
}
}

@Test
public void horizontalPanBoundsAreCachedAndInvalidatedByTextChanges() {
setEditorPreferences(true, false);

try (ActivityScenario<EditorActivity> 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);
Expand Down
83 changes: 66 additions & 17 deletions app/src/main/java/com/maxistar/textpad/activities/EditorActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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;

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Cancel EditText touch handling when owning a pan

When the second finger makes the listener own the gesture, EditText.onTouchEvent has already received the initial ACTION_DOWN, but this listener now consumes every remaining event, including the final ACTION_UP or ACTION_CANCEL. The editor therefore never clears its pending tap/long-press and pressed state, so a quick two-finger pan can trigger a delayed long-click or selection at the first touch location after both fingers are released. Send the editor a cancellation event when taking ownership before swallowing the rest of the stream.

Useful? React with 👍 / 👎.

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) {
Expand Down Expand Up @@ -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;
Comment on lines +2117 to +2120

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Invalidate cached bounds after background reloads

When an unchanged open document is modified externally while the activity is paused, onResume calls validateOpenDocumentOnForeground() before reattaching textWatcher, so the resulting setEditorText() never invokes the new invalidation in onTextChanged. If the replacement changes the longest line without changing the editor's dimensions, this condition reuses the old maximum and horizontal panning either cannot reach the new text or scrolls beyond shorter text until another edit, preference change, or resize invalidates the cache.

Useful? React with 👍 / 👎.

}
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) {
Expand Down