Skip to content
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.

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

feat: updated prevent typing over-long topic names #1303

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
63 changes: 53 additions & 10 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import 'text.dart';
import 'theme.dart';

const double _composeButtonSize = 44;
const int kMaxTopicLength = 60;

/// A [TextEditingController] for use in the compose box.
///
Expand Down Expand Up @@ -90,12 +91,27 @@ enum TopicValidationError {

class ComposeTopicController extends ComposeController<TopicValidationError> {
ComposeTopicController() {
addListener(_enforceCharacterLimit);
_update();
}

static final characterCount = ValueNotifier<int>(0);
// TODO: subscribe to this value:
// https://zulip.com/help/require-topics
final mandatory = true;
void _enforceCharacterLimit() {
if (text.length > kMaxTopicLength) {
// Truncate text to `kMaxTopicLength`
final newText = text.substring(0, kMaxTopicLength);

// Update controller value and selection (sync with TextField)
value = value.copyWith(
text: newText,
selection: TextSelection.collapsed(offset: newText.length),
);
}
// Update the character count
characterCount.value = text.length.clamp(0, kMaxTopicLength);
}

// TODO(#307) use `max_topic_length` instead of hardcoded limit
@override final maxLengthUnicodeCodePoints = kMaxTopicLengthCodePoints;
Expand Down Expand Up @@ -555,15 +571,42 @@ class _TopicInput extends StatelessWidget {
decoration: BoxDecoration(border: Border(bottom: BorderSide(
width: 1,
color: designVariables.foreground.withFadedAlpha(0.2)))),
child: TextField(
controller: controller.topic,
focusNode: controller.topicFocusNode,
textInputAction: TextInputAction.next,
style: topicTextStyle,
decoration: InputDecoration(
hintText: zulipLocalizations.composeBoxTopicHintText,
hintStyle: topicTextStyle.copyWith(
color: designVariables.textInput.withFadedAlpha(0.5))))));
child: Column(
crossAxisAlignment: CrossAxisAlignment.start, // Aligns text elements to the start
children: [
ValueListenableBuilder<int>(
valueListenable: ComposeTopicController.characterCount,
builder: (context, count, child) {
return Text(
'$count / $kMaxTopicLength',
style: TextStyle(
fontSize: 12,
color: count >= kMaxTopicLength
? designVariables.btnLabelAttMediumIntDanger
: designVariables.foreground.withFadedAlpha(0.5),
),
);
},
),
TextField(
controller: controller.topic,
focusNode: controller.topicFocusNode,
textInputAction: TextInputAction.next,
style: topicTextStyle,
decoration: InputDecoration(
contentPadding: EdgeInsets.zero, // Removes any padding
hintText: zulipLocalizations.composeBoxTopicHintText,
hintStyle: topicTextStyle.copyWith(
color: designVariables.textInput.withFadedAlpha(0.5),
),
),
inputFormatters: [
LengthLimitingTextInputFormatter(kMaxTopicLength),
],
),

],
)));
}
}

Expand Down
126 changes: 126 additions & 0 deletions test/widgets/compose_box_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'dart:io';
import 'package:checks/checks.dart';
import 'package:file_picker/file_picker.dart';
import 'package:flutter_checks/flutter_checks.dart';
import 'package:flutter/services.dart';
import 'package:http/http.dart' as http;
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
Expand Down Expand Up @@ -560,6 +561,131 @@ void main() {
});
});

group('ComposeTopicController', () {
const kMaxTopicLength = 60; // Example max length
const longTopic = "This is a sample string that is exactly sixty characters long. This is a very long topic that exceeds the maximum allowed length. ";
const trimmedLongTopic = "This is a sample string that is exactly sixty characters lon"; // Truncated

late ComposeTopicController controller;

setUp(() {
controller = ComposeTopicController();
});

tearDown(() {
controller.dispose();
});

test('enforces character limit when text exceeds kMaxTopicLength', () {
controller.text = longTopic;

// Verify truncation logic
expect(controller.text, trimmedLongTopic);
expect(ComposeTopicController.characterCount.value, kMaxTopicLength);
});

test('updates character count correctly', () {
controller.text = trimmedLongTopic;

// Verify character count matches the topic length
expect(
ComposeTopicController.characterCount.value, trimmedLongTopic.length);
});

test('detects mandatory topic violation', () {
controller.text = ""; // Empty text
final errors = controller.validationErrors;

expect(errors, contains(TopicValidationError.mandatoryButEmpty));
});

});

group('ComposeTopic UI', () {
testWidgets('displays correct character count while typing', (
WidgetTester tester) async {
const kMaxTopicLength = 60;
final controller = ComposeTopicController();

await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Column(
children: [
ValueListenableBuilder<int>(
valueListenable: ComposeTopicController.characterCount,
builder: (context, count, child) {
return Text('$count / $kMaxTopicLength');
},
),
TextField(
controller: controller,
inputFormatters: [
LengthLimitingTextInputFormatter(kMaxTopicLength),
],
),
],
),
),
),
);

final textFieldFinder = find.byType(TextField);
final characterCountFinder = find.text('60 / $kMaxTopicLength');

// Initial state
expect(characterCountFinder, findsOneWidget);

// Enter valid text
await tester.enterText(textFieldFinder, 'Test topic');
await tester.pump();

// Updated state
expect(find.text('10 / $kMaxTopicLength'), findsOneWidget);
});

testWidgets(
'displays truncated text and updated count when exceeding max length',
(WidgetTester tester) async {
const kMaxTopicLength = 60;
final controller = ComposeTopicController();
const longTopic = "This is a sample string that is exactly sixty characters long. This is a very long topic that exceeds the maximum allowed length. ";
const trimmedLongTopic = "This is a sample string that is exactly sixty characters lon"; //Truncated
await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Column(
children: [
ValueListenableBuilder<int>(
valueListenable: ComposeTopicController.characterCount,
builder: (context, count, child) {
return Text('$count / $kMaxTopicLength');
},
),
TextField(
controller: controller,
inputFormatters: [
LengthLimitingTextInputFormatter(kMaxTopicLength),
],
),
],
),
),
),
);

final textFieldFinder = find.byType(TextField);

// Enter long text
await tester.enterText(textFieldFinder, longTopic);
await tester.pump();

expect(controller.text, trimmedLongTopic);
expect(
find.text('$kMaxTopicLength / $kMaxTopicLength'), findsOneWidget);
});
});

group('uploads', () {
void checkAppearsLoading(WidgetTester tester, bool expected) {
final sendButtonElement = tester.element(find.ancestor(
Expand Down
Loading