fix: Don't rely on removeDiacritics to highlight text (#4636)

* When we highlight a suggestion, we can't rely on the `removeDiacritics`, as it replaces some letters of 1 word into 2 or + * Better algorithm * Remove unused things * Fix typo * Unnecessary brackets * Replace all `toLowerCase` + `removeDiacritics` by a single `getComparisonSafeString` * Reformat CountrySelector * `removeAccents` removed + 1 new test --------- Co-authored-by: monsieurtanuki <[email protected]>
openfoodfacts · Dec 16, 2023 · e904b05 · e904b05
1 parent f81ba98
commit e904b05
Show file tree

Hide file tree

Showing 6 changed files with 128 additions and 39 deletions.
diff --git a/packages/smooth_app/lib/generic_lib/widgets/language_selector.dart b/packages/smooth_app/lib/generic_lib/widgets/language_selector.dart
@@ -1,4 +1,3 @@
-import 'package:diacritic/diacritic.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_gen/gen_l10n/app_localizations.dart';
 import 'package:openfoodfacts/openfoodfacts.dart';
@@ -146,19 +145,19 @@ class LanguageSelector extends StatelessWidget {
             prefixIcon: const Icon(Icons.search),
             controller: languageSelectorController,
             onChanged: (String? query) {
-              query = removeDiacritics(query!.trim().toLowerCase());
+              query = query!.trim().getComparisonSafeString();
 
               setState(
                 () {
                   filteredList = leftovers
                       .where((OpenFoodFactsLanguage item) =>
-                          removeDiacritics(_languages
-                                  .getNameInEnglish(item)
-                                  .toLowerCase())
+                          _languages
+                              .getNameInEnglish(item)
+                              .getComparisonSafeString()
                               .contains(query!.toLowerCase()) ||
-                          removeDiacritics(_languages
-                                  .getNameInLanguage(item)
-                                  .toLowerCase())
+                          _languages
+                              .getNameInLanguage(item)
+                              .getComparisonSafeString()
                               .contains(query.toLowerCase()) ||
                           item.code.contains(query))
                       .toList();

diff --git a/packages/smooth_app/lib/pages/onboarding/country_selector.dart b/packages/smooth_app/lib/pages/onboarding/country_selector.dart
@@ -1,4 +1,3 @@
-import 'package:diacritic/diacritic.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
 import 'package:flutter_gen/gen_l10n/app_localizations.dart';
@@ -104,23 +103,23 @@ class _CountrySelectorState extends State<CountrySelector> {
                         prefixIcon: const Icon(Icons.search),
                         controller: _countryController,
                         onChanged: (String? query) {
-                          query = removeDiacritics(query!.trim().toLowerCase());
+                          query = query!.trim()..getComparisonSafeString();
 
                           setState(
                             () {
                               filteredList = _countryList
                                   .where(
                                     (Country item) =>
-                                        removeDiacritics(
-                                                item.name.toLowerCase())
+                                        item.name
+                                            .getComparisonSafeString()
                                             .contains(
-                                          query!,
-                                        ) ||
-                                        removeDiacritics(
-                                                item.countryCode.toLowerCase())
+                                              query!,
+                                            ) ||
+                                        item.countryCode
+                                            .getComparisonSafeString()
                                             .contains(
-                                          query,
-                                        ),
+                                              query,
+                                            ),
                                   )
                                   .toList(growable: false);
                             },

diff --git a/packages/smooth_app/lib/pages/preferences/user_preferences_search_page.dart b/packages/smooth_app/lib/pages/preferences/user_preferences_search_page.dart
@@ -1,11 +1,11 @@
-import 'package:diacritic/diacritic.dart';
 import 'package:flutter/material.dart';
 import 'package:provider/provider.dart';
 import 'package:smooth_app/data_models/preferences/user_preferences.dart';
 import 'package:smooth_app/generic_lib/design_constants.dart';
 import 'package:smooth_app/pages/preferences/abstract_user_preferences.dart';
 import 'package:smooth_app/pages/preferences/user_preferences_item.dart';
 import 'package:smooth_app/pages/preferences/user_preferences_page.dart';
+import 'package:smooth_app/widgets/smooth_text.dart';
 
 /// Search page for preferences, with TextField filter.
 class UserPreferencesSearchPage extends StatefulWidget {
@@ -70,7 +70,7 @@ class _UserPreferencesSearchPageState extends State<UserPreferencesSearchPage> {
     final String searchString,
     final UserPreferences userPreferences,
   ) {
-    final String needle = removeDiacritics(searchString.toLowerCase());
+    final String needle = searchString.getComparisonSafeString();
     final List<Widget> result = <Widget>[];
     final List<PreferencePageType> types =
         PreferencePageType.getPreferencePageTypes(userPreferences);
@@ -101,7 +101,7 @@ class _UserPreferencesSearchPageState extends State<UserPreferencesSearchPage> {
 
   bool _findLabels(final String needle, final Iterable<String> labels) {
     for (final String label in labels) {
-      if (removeDiacritics(label.toLowerCase()).contains(needle)) {
+      if (label.getComparisonSafeString().contains(needle)) {
         return true;
       }
     }

diff --git a/packages/smooth_app/lib/pages/product/nutrition_add_nutrient_button.dart b/packages/smooth_app/lib/pages/product/nutrition_add_nutrient_button.dart
@@ -1,4 +1,3 @@
-import 'package:diacritic/diacritic.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_gen/gen_l10n/app_localizations.dart';
 import 'package:openfoodfacts/openfoodfacts.dart';
@@ -55,9 +54,14 @@ class NutritionAddNutrientButton extends StatelessWidget {
                 controller: nutritionTextController,
                 onChanged: (String? query) => setState(
                   () => filteredList = leftovers
-                      .where((OrderedNutrient item) =>
-                          removeDiacritics(item.name!).toLowerCase().contains(
-                              removeDiacritics(query!).toLowerCase().trim()))
+                      .where(
+                        (OrderedNutrient item) => item.name!
+                            .trim()
+                            .getComparisonSafeString()
+                            .contains(
+                              query!.trim().getComparisonSafeString(),
+                            ),
+                      )
                       .toList(),
                 ),
               ),

diff --git a/packages/smooth_app/lib/widgets/smooth_text.dart b/packages/smooth_app/lib/widgets/smooth_text.dart
@@ -1,5 +1,21 @@
-import 'package:diacritic/diacritic.dart';
+import 'package:diacritic/diacritic.dart' as lib show removeDiacritics;
 import 'package:flutter/material.dart';
+import 'package:smooth_app/services/smooth_services.dart';
+
+/// An extension on [String]
+extension StringExtension on String {
+  /// Please use this method instead of directly calling the library.
+  /// It will ease the migration if we decide to remove/change it.
+  String removeDiacritics() {
+    return lib.removeDiacritics(this);
+  }
+
+  /// Same as [removeDiacritics] but also lowercases the string.
+  /// Prefer this method when you want to compare two strings.
+  String getComparisonSafeString() {
+    return toLowerCase().removeDiacritics();
+  }
+}
 
 /// An extension on [TextStyle] that allows to have "well spaced" variant
 extension TextStyleExtension on TextStyle {
@@ -60,13 +76,23 @@ class TextHighlighter extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    final List<(String, TextStyle?)> parts = _getParts(
-      defaultStyle: TextStyle(fontWeight: selected ? FontWeight.bold : null),
-      highlightedStyle: TextStyle(
-        fontWeight: selected ? FontWeight.bold : null,
-        backgroundColor: Theme.of(context).primaryColor.withOpacity(0.2),
-      ),
-    );
+    List<(String, TextStyle?)> parts;
+    try {
+      parts = _getParts(
+        defaultStyle: TextStyle(fontWeight: selected ? FontWeight.bold : null),
+        highlightedStyle: TextStyle(
+          fontWeight: selected ? FontWeight.bold : null,
+          backgroundColor: Theme.of(context).primaryColor.withOpacity(0.2),
+        ),
+      );
+    } catch (e, trace) {
+      parts = <(String, TextStyle?)>[(text, null)];
+      Logs.e(
+        'Unable to parse text "$text" with filter "$filter".',
+        ex: e,
+        stacktrace: trace,
+      );
+    }
 
     final TextStyle defaultTextStyle = DefaultTextStyle.of(context).style;
 
@@ -91,9 +117,12 @@ class TextHighlighter extends StatelessWidget {
     required TextStyle? defaultStyle,
     required TextStyle? highlightedStyle,
   }) {
+    final String filterWithoutDiacritics = filter.getComparisonSafeString();
+    final String textWithoutDiacritics = text.getComparisonSafeString();
+
     final Iterable<RegExpMatch> highlightedParts =
-        RegExp(removeDiacritics(filter).toLowerCase().trim()).allMatches(
-      removeDiacritics(text).toLowerCase(),
+        RegExp(filterWithoutDiacritics.trim()).allMatches(
+      textWithoutDiacritics,
     );
 
     final List<(String, TextStyle?)> parts = <(String, TextStyle?)>[];
@@ -103,24 +132,61 @@ class TextHighlighter extends StatelessWidget {
     } else {
       parts
           .add((text.substring(0, highlightedParts.first.start), defaultStyle));
+      int diff = 0;
+
       for (int i = 0; i != highlightedParts.length; i++) {
         final RegExpMatch subPart = highlightedParts.elementAt(i);
+        final int startPosition = subPart.start - diff;
+        final int endPosition = _computeEndPosition(
+          startPosition,
+          subPart.end - diff,
+          subPart,
+          textWithoutDiacritics,
+          filterWithoutDiacritics,
+        );
+        diff = subPart.end - endPosition;
 
         parts.add(
-          (text.substring(subPart.start, subPart.end), highlightedStyle),
+          (text.substring(startPosition, endPosition), highlightedStyle),
         );
 
         if (i < highlightedParts.length - 1) {
           parts.add((
             text.substring(
-                subPart.end, highlightedParts.elementAt(i + 1).start),
+                endPosition, highlightedParts.elementAt(i + 1).start - diff),
             defaultStyle
           ));
-        } else if (subPart.end < text.length) {
-          parts.add((text.substring(subPart.end, text.length), defaultStyle));
+        } else if (endPosition < text.length) {
+          parts.add((text.substring(endPosition, text.length), defaultStyle));
         }
       }
     }
     return parts;
   }
+
+  int _computeEndPosition(
+    int startPosition,
+    int endPosition,
+    RegExpMatch subPart,
+    String textWithoutDiacritics,
+    String filterWithoutDiacritics,
+  ) {
+    final String subText = text.substring(startPosition);
+    if (subText.startsWith(filterWithoutDiacritics)) {
+      return endPosition;
+    }
+
+    int diff = 0;
+    for (int pos = 0; pos < endPosition; pos++) {
+      if (pos == subText.length - 1) {
+        diff = pos - subText.length;
+        break;
+      }
+
+      final int charLength = subText[pos].removeDiacritics().length;
+      diff -= charLength > 1 ? charLength - 1 : 0;
+    }
+
+    return endPosition + diff;
+  }
 }
diff --git a/packages/smooth_app/test/utils/smooth_text_test.dart b/packages/smooth_app/test/utils/smooth_text_test.dart
@@ -0,0 +1,21 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:smooth_app/widgets/smooth_text.dart' show StringExtension;
+
+void main() {
+  group('Smooth text', () {
+    group('String extension', () {
+      test('Remove diacritics (oeuf)', () {
+        expect(
+          'œuf'.removeDiacritics(),
+          equals('oeuf'),
+        );
+      });
+      test('Comparison Safe String', () {
+        expect(
+          'œuF'.getComparisonSafeString(),
+          equals('oeuf'),
+        );
+      });
+    });
+  });
+}