Skip to content

Fixed cursor jumping to end issue #925

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

Open
tusharonly wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fixed cursor jumping to end issue #925

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22ed22a
FIx EnvironmentTriggerFieldState: Preserve cursor position when updat…
tusharonly Nov 21, 2025
933b401
Update env_trigger_field.dart
animator Dec 4, 2025
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
27 changes: 22 additions & 5 deletions lib/screens/common_widgets/env_trigger_field.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class EnvironmentTriggerField extends StatefulWidget {
this.optionsWidthFactor,
this.autocompleteNoTrigger,
this.readOnly = false,
this.obscureText = false
this.obscureText = false,
}) : assert(
!(controller != null && initialValue != null),
'controller and initialValue cannot be simultaneously defined.',
Expand Down Expand Up @@ -67,13 +67,31 @@ class EnvironmentTriggerFieldState extends State<EnvironmentTriggerField> {
@override
void didUpdateWidget(EnvironmentTriggerField oldWidget) {
super.didUpdateWidget(oldWidget);
if ((oldWidget.keyId != widget.keyId) ||
(oldWidget.initialValue != widget.initialValue)) {
if (oldWidget.keyId != widget.keyId) {
// Key changed - create new controller with cursor at end
controller = widget.controller ??
TextEditingController.fromValue(TextEditingValue(
text: widget.initialValue!,
selection: TextSelection.collapsed(
offset: widget.initialValue!.length)));
Comment on lines +70 to 76
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

Creating a new controller without disposing the old one will cause a memory leak. When the keyId changes, the old controller should be disposed before assigning a new one.

Consider adding:

if (oldWidget.keyId != widget.keyId) {
  // Key changed - dispose old controller and create new one
  if (widget.controller == null) {
    controller.dispose();
  }
  controller = widget.controller ??
      TextEditingController.fromValue(TextEditingValue(
          text: widget.initialValue!,
          selection: TextSelection.collapsed(
              offset: widget.initialValue!.length)));
}

Copilot uses AI. Check for mistakes.
} else if (oldWidget.initialValue != widget.initialValue) {
// Initial value changed but key is same
// Preserve cursor position if text is being updated
if (widget.controller == null) {
final currentSelection = controller.selection;
final newText = widget.initialValue ?? '';

// Only update if the text actually differs from controller's current text
if (controller.text != newText) {
// Preserve cursor position, ensuring it's within bounds
final newOffset =
currentSelection.baseOffset.clamp(0, newText.length);
controller.value = TextEditingValue(
text: newText,
selection: TextSelection.collapsed(offset: newOffset),
Comment on lines +87 to +91
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The cursor position preservation logic doesn't account for text selection ranges (when text is highlighted). The code only preserves baseOffset but ignores extentOffset, which means any text selection will be lost and collapsed to a cursor position.

Consider preserving the full selection when possible:

final currentSelection = controller.selection;
final newText = widget.initialValue ?? '';

if (controller.text != newText) {
  // Preserve selection range, ensuring it's within bounds
  final newBaseOffset = currentSelection.baseOffset.clamp(0, newText.length);
  final newExtentOffset = currentSelection.extentOffset.clamp(0, newText.length);
  
  controller.value = TextEditingValue(
    text: newText,
    selection: TextSelection(
      baseOffset: newBaseOffset,
      extentOffset: newExtentOffset,
    ),
  );
}
Suggested change
final newOffset =
currentSelection.baseOffset.clamp(0, newText.length);
controller.value = TextEditingValue(
text: newText,
selection: TextSelection.collapsed(offset: newOffset),
final newBaseOffset = currentSelection.baseOffset.clamp(0, newText.length);
final newExtentOffset = currentSelection.extentOffset.clamp(0, newText.length);
controller.value = TextEditingValue(
text: newText,
selection: TextSelection(
baseOffset: newBaseOffset,
extentOffset: newExtentOffset,
),

Copilot uses AI. Check for mistakes.
);
}
}
Comment on lines +77 to +94
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The cursor position preservation logic introduced in this PR lacks test coverage. While there are existing tests that verify text updates, there are no tests that specifically validate that the cursor position is preserved when text is updated (the main fix of this PR).

Consider adding a test case like:

testWidgets('Testing EnvironmentTriggerField preserves cursor position on text update',
    (WidgetTester tester) async {
  final fieldKey = GlobalKey<EnvironmentTriggerFieldState>();
  
  await tester.pumpWidget(
    Portal(
      child: MaterialApp(
        home: Scaffold(
          body: EnvironmentTriggerField(
            key: fieldKey,
            keyId: 'testKey',
            initialValue: 'hello world',
          ),
        ),
      ),
    ),
  );
  
  // Set cursor to position 5 (middle of text)
  fieldKey.currentState!.controller.selection = 
      TextSelection.collapsed(offset: 5);
  
  // Update the text value
  await tester.pumpWidget(
    Portal(
      child: MaterialApp(
        home: Scaffold(
          body: EnvironmentTriggerField(
            key: fieldKey,
            keyId: 'testKey',
            initialValue: 'hello world!',
          ),
        ),
      ),
    ),
  );
  
  // Verify cursor is still at position 5
  expect(fieldKey.currentState!.controller.selection.baseOffset, 5);
});

Copilot uses AI. Check for mistakes.
}
}

Expand Down Expand Up @@ -130,8 +148,7 @@ class EnvironmentTriggerFieldState extends State<EnvironmentTriggerField> {
_focusNode.unfocus();
},
readOnly: widget.readOnly,
obscureText: widget.obscureText

obscureText: widget.obscureText,
);
},
);
Expand Down