From f5cb8b6d250a1c6c4e2e477d5e51808c1e3822db Mon Sep 17 00:00:00 2001 From: Mathias Mogensen <42929161+Xazin@users.noreply.github.com> Date: Wed, 6 Mar 2024 16:31:30 +0100 Subject: [PATCH] fix: image url check for embed link (#4826) * fix: image url check in for embed link * chore: move all patterns to shared * test: prefer enterText over manipulating widget --- .../shared/editor_test_operations.dart | 8 +++--- .../setting/font/font_picker_screen.dart | 23 +++++++--------- .../widgets/calculations/calculate_cell.dart | 6 ++--- .../copy_and_paste/paste_from_plain_text.dart | 9 +++---- .../image/embed_image_url_widget.dart | 26 ++++++++++++++++--- .../mobile_toolbar_v3/_font_item.dart | 3 ++- .../lib/shared/patterns/common_patterns.dart | 23 ++++++++++++++++ .../shared/patterns/date_time_patterns.dart | 19 ++++++++++++++ .../util/google_font_family_extension.dart | 9 +++---- .../lib/util/string_extension.dart | 8 +++--- .../settings/application_data_storage.dart | 6 +++-- .../settings/date_time/time_patterns.dart | 18 ------------- .../font_family_setting.dart | 21 +++++---------- 13 files changed, 104 insertions(+), 75 deletions(-) create mode 100644 frontend/appflowy_flutter/lib/shared/patterns/common_patterns.dart create mode 100644 frontend/appflowy_flutter/lib/shared/patterns/date_time_patterns.dart delete mode 100644 frontend/appflowy_flutter/lib/workspace/application/settings/date_time/time_patterns.dart diff --git a/frontend/appflowy_flutter/integration_test/shared/editor_test_operations.dart b/frontend/appflowy_flutter/integration_test/shared/editor_test_operations.dart index 64dbd1746e..786b9e08ce 100644 --- a/frontend/appflowy_flutter/integration_test/shared/editor_test_operations.dart +++ b/frontend/appflowy_flutter/integration_test/shared/editor_test_operations.dart @@ -1,6 +1,9 @@ import 'dart:async'; import 'dart:ui'; +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; + import 'package:appflowy/generated/locale_keys.g.dart'; import 'package:appflowy/plugins/base/emoji/emoji_picker.dart'; import 'package:appflowy/plugins/base/emoji/emoji_skin_tone.dart'; @@ -14,8 +17,6 @@ import 'package:appflowy/plugins/document/presentation/editor_plugins/image/embe import 'package:appflowy/plugins/inline_actions/widgets/inline_actions_handler.dart'; import 'package:appflowy_editor/appflowy_editor.dart' hide Log; import 'package:easy_localization/easy_localization.dart'; -import 'package:flutter/material.dart'; -import 'package:flutter/services.dart'; import 'package:flutter_emoji_mart/flutter_emoji_mart.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -132,8 +133,7 @@ class EditorOperations { of: find.byType(EmbedImageUrlWidget), matching: find.byType(TextField), ); - final textField = tester.widget(imageUrlTextField); - textField.controller?.text = imageUrl; + await tester.enterText(imageUrlTextField, imageUrl); await tester.pumpAndSettle(); await tester.tapButton( find.descendant( diff --git a/frontend/appflowy_flutter/lib/mobile/presentation/setting/font/font_picker_screen.dart b/frontend/appflowy_flutter/lib/mobile/presentation/setting/font/font_picker_screen.dart index a2a6ca04f5..f79bc3dd78 100644 --- a/frontend/appflowy_flutter/lib/mobile/presentation/setting/font/font_picker_screen.dart +++ b/frontend/appflowy_flutter/lib/mobile/presentation/setting/font/font_picker_screen.dart @@ -1,10 +1,12 @@ +import 'package:flutter/material.dart'; + import 'package:appflowy/generated/locale_keys.g.dart'; import 'package:appflowy/mobile/presentation/base/app_bar.dart'; import 'package:appflowy/mobile/presentation/widgets/flowy_mobile_search_text_field.dart'; import 'package:appflowy/mobile/presentation/widgets/widgets.dart'; +import 'package:appflowy/util/google_font_family_extension.dart'; import 'package:appflowy/workspace/application/settings/appearance/appearance_cubit.dart'; import 'package:easy_localization/easy_localization.dart'; -import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:go_router/go_router.dart'; import 'package:google_fonts/google_fonts.dart'; @@ -23,9 +25,7 @@ class FontPickerScreen extends StatelessWidget { } class LanguagePickerPage extends StatefulWidget { - const LanguagePickerPage({ - super.key, - }); + const LanguagePickerPage({super.key}); @override State createState() => _LanguagePickerPageState(); @@ -52,6 +52,7 @@ class _LanguagePickerPageState extends State { body: SafeArea( child: Scrollbar( child: ListView.builder( + itemCount: availableFonts.length + 1, // with search bar itemBuilder: (context, index) { if (index == 0) { // search bar @@ -65,7 +66,8 @@ class _LanguagePickerPageState extends State { setState(() { availableFonts = _availableFonts .where( - (element) => parseFontFamilyName(element) + (font) => font + .parseFontFamilyName() .toLowerCase() .contains(keyword.toLowerCase()), ) @@ -75,8 +77,9 @@ class _LanguagePickerPageState extends State { ), ); } + final fontFamilyName = availableFonts[index - 1]; - final displayName = parseFontFamilyName(fontFamilyName); + final displayName = fontFamilyName.parseFontFamilyName(); return FlowyOptionTile.checkbox( text: displayName, isSelected: selectedFontFamilyName == fontFamilyName, @@ -86,17 +89,9 @@ class _LanguagePickerPageState extends State { backgroundColor: Colors.transparent, ); }, - itemCount: availableFonts.length + 1, // with search bar ), ), ), ); } - - String parseFontFamilyName(String fontFamilyName) { - final camelCase = RegExp('(?<=[a-z])[A-Z]'); - return fontFamilyName - .replaceAll('_regular', '') - .replaceAllMapped(camelCase, (m) => ' ${m.group(0)}'); - } } diff --git a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/calculations/calculate_cell.dart b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/calculations/calculate_cell.dart index 9b607fc712..2e73e72e12 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/calculations/calculate_cell.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/calculations/calculate_cell.dart @@ -9,6 +9,7 @@ import 'package:appflowy/plugins/database/grid/application/calculations/field_ty import 'package:appflowy/plugins/database/grid/presentation/widgets/calculations/calculation_selector.dart'; import 'package:appflowy/plugins/database/grid/presentation/widgets/calculations/calculation_type_item.dart'; import 'package:appflowy/plugins/database/grid/presentation/widgets/calculations/remove_calculation_button.dart'; +import 'package:appflowy/shared/patterns/common_patterns.dart'; import 'package:appflowy_backend/protobuf/flowy-database2/calculation_entities.pb.dart'; import 'package:appflowy_backend/protobuf/flowy-database2/field_entities.pbenum.dart'; import 'package:appflowy_backend/protobuf/flowy-database2/number_entities.pb.dart'; @@ -132,9 +133,8 @@ class _CalculateCellState extends State { } String _withoutTrailingZeros(String value) { - final regex = RegExp(r'^(\d+(?:\.\d*?[1-9](?=0|\b))?)\.?0*$'); - if (regex.hasMatch(value)) { - final match = regex.firstMatch(value)!; + if (trailingZerosRegex.hasMatch(value)) { + final match = trailingZerosRegex.firstMatch(value)!; return match.group(1)!; } diff --git a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/copy_and_paste/paste_from_plain_text.dart b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/copy_and_paste/paste_from_plain_text.dart index 7f31309fc6..114dde62a3 100644 --- a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/copy_and_paste/paste_from_plain_text.dart +++ b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/copy_and_paste/paste_from_plain_text.dart @@ -1,10 +1,7 @@ import 'package:appflowy/plugins/document/presentation/editor_plugins/copy_and_paste/editor_state_paste_node_extension.dart'; +import 'package:appflowy/shared/patterns/common_patterns.dart'; import 'package:appflowy_editor/appflowy_editor.dart'; -RegExp _hrefRegex = RegExp( - r'https?://(?:www\.)?[a-zA-Z0-9\-\.]+\.[a-zA-Z]{2,}(?:/[^\s]*)?', -); - extension PasteFromPlainText on EditorState { Future pastePlainText(String plainText) async { if (await pasteHtmlIfAvailable(plainText)) { @@ -23,7 +20,7 @@ extension PasteFromPlainText on EditorState { .map((e) { // parse the url content final Attributes attributes = {}; - if (_hrefRegex.hasMatch(e)) { + if (hrefRegex.hasMatch(e)) { attributes[AppFlowyRichTextKeys.href] = e; } return Delta()..insert(e, attributes: attributes); @@ -45,7 +42,7 @@ extension PasteFromPlainText on EditorState { if (selection == null || !selection.isSingle || selection.isCollapsed || - !_hrefRegex.hasMatch(plainText)) { + !hrefRegex.hasMatch(plainText)) { return false; } diff --git a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/embed_image_url_widget.dart b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/embed_image_url_widget.dart index 34400597ac..d34a4fc3a8 100644 --- a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/embed_image_url_widget.dart +++ b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/embed_image_url_widget.dart @@ -1,7 +1,9 @@ +import 'package:flutter/material.dart'; + import 'package:appflowy/generated/locale_keys.g.dart'; +import 'package:appflowy/shared/patterns/common_patterns.dart'; import 'package:easy_localization/easy_localization.dart'; import 'package:flowy_infra_ui/flowy_infra_ui.dart'; -import 'package:flutter/material.dart'; class EmbedImageUrlWidget extends StatefulWidget { const EmbedImageUrlWidget({ @@ -16,6 +18,7 @@ class EmbedImageUrlWidget extends StatefulWidget { } class _EmbedImageUrlWidgetState extends State { + bool isUrlValid = true; String inputText = ''; @override @@ -25,8 +28,15 @@ class _EmbedImageUrlWidgetState extends State { FlowyTextField( hintText: LocaleKeys.document_imageBlock_embedLink_placeholder.tr(), onChanged: (value) => inputText = value, - onEditingComplete: () => widget.onSubmit(inputText), + onEditingComplete: submit, ), + if (!isUrlValid) ...[ + const VSpace(8), + FlowyText( + LocaleKeys.document_plugins_cover_invalidImageUrl.tr(), + color: Theme.of(context).colorScheme.error, + ), + ], const VSpace(8), SizedBox( width: 160, @@ -37,10 +47,20 @@ class _EmbedImageUrlWidgetState extends State { LocaleKeys.document_imageBlock_embedLink_label.tr(), textAlign: TextAlign.center, ), - onTap: () => widget.onSubmit(inputText), + onTap: submit, ), ), ], ); } + + void submit() { + if (checkUrlValidity(inputText)) { + return widget.onSubmit(inputText); + } + + setState(() => isUrlValid = false); + } + + bool checkUrlValidity(String url) => imgUrlRegex.hasMatch(url); } diff --git a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/mobile_toolbar_v3/_font_item.dart b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/mobile_toolbar_v3/_font_item.dart index cc5914f53b..13384f9b87 100644 --- a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/mobile_toolbar_v3/_font_item.dart +++ b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/mobile_toolbar_v3/_font_item.dart @@ -1,12 +1,13 @@ import 'dart:async'; +import 'package:flutter/material.dart'; + import 'package:appflowy/mobile/presentation/setting/font/font_picker_screen.dart'; import 'package:appflowy/plugins/document/application/document_appearance_cubit.dart'; import 'package:appflowy/plugins/document/presentation/editor_plugins/mobile_toolbar_v3/_toolbar_theme.dart'; import 'package:appflowy/plugins/document/presentation/editor_plugins/plugins.dart'; import 'package:appflowy/util/google_font_family_extension.dart'; import 'package:appflowy_editor/appflowy_editor.dart'; -import 'package:flutter/material.dart'; import 'package:go_router/go_router.dart'; import 'package:google_fonts/google_fonts.dart'; import 'package:provider/provider.dart'; diff --git a/frontend/appflowy_flutter/lib/shared/patterns/common_patterns.dart b/frontend/appflowy_flutter/lib/shared/patterns/common_patterns.dart new file mode 100644 index 0000000000..fce937908a --- /dev/null +++ b/frontend/appflowy_flutter/lib/shared/patterns/common_patterns.dart @@ -0,0 +1,23 @@ +const _trailingZerosPattern = r'^(\d+(?:\.\d*?[1-9](?=0|\b))?)\.?0*$'; +final trailingZerosRegex = RegExp(_trailingZerosPattern); + +const _hrefPattern = + r'https?://(?:www\.)?[a-zA-Z0-9\-\.]+\.[a-zA-Z]{2,}(?:/[^\s]*)?'; +final hrefRegex = RegExp(_hrefPattern); + +/// This pattern allows for both HTTP and HTTPS Scheme +/// It allows for query parameters +/// It only allows the following image extensions: .png, .jpg, .gif, .webm +/// +const _imgUrlPattern = + r'(https?:\/\/)([^\s(["<,>/]*)(\/)[^\s[",><]*(.png|.jpg|.gif|.webm)(\?[^\s[",><]*)?'; +final imgUrlRegex = RegExp(_imgUrlPattern); + +const _appflowyCloudUrlPattern = r'^(https:\/\/)(.*)(\.appflowy\.cloud\/)(.*)'; +final appflowyCloudUrlRegex = RegExp(_appflowyCloudUrlPattern); + +const _camelCasePattern = '(?<=[a-z])[A-Z]'; +final camelCaseRegex = RegExp(_camelCasePattern); + +const _macOSVolumesPattern = '^/Volumes/[^/]+'; +final macOSVolumesRegex = RegExp(_macOSVolumesPattern); diff --git a/frontend/appflowy_flutter/lib/shared/patterns/date_time_patterns.dart b/frontend/appflowy_flutter/lib/shared/patterns/date_time_patterns.dart new file mode 100644 index 0000000000..530e9d4558 --- /dev/null +++ b/frontend/appflowy_flutter/lib/shared/patterns/date_time_patterns.dart @@ -0,0 +1,19 @@ +/// RegExp to match Twelve Hour formats +/// Source: https://stackoverflow.com/a/33906224 +/// +/// Matches eg: "05:05 PM", "5:50 Pm", "10:59 am", etc. +/// +const _twelveHourTimePattern = + r'\b((1[0-2]|0?[1-9]):([0-5][0-9]) ([AaPp][Mm]))'; +final twelveHourTimeRegex = RegExp(_twelveHourTimePattern); +bool isTwelveHourTime(String? time) => twelveHourTimeRegex.hasMatch(time ?? ''); + +/// RegExp to match Twenty Four Hour formats +/// Source: https://stackoverflow.com/a/7536768 +/// +/// Matches eg: "0:01", "04:59", "16:30", etc. +/// +const _twentyFourHourtimePattern = r'^([0-1]?[0-9]|2[0-3]):[0-5][0-9]$'; +final tewentyFourHourTimeRegex = RegExp(_twentyFourHourtimePattern); +bool isTwentyFourHourTime(String? time) => + tewentyFourHourTimeRegex.hasMatch(time ?? ''); diff --git a/frontend/appflowy_flutter/lib/util/google_font_family_extension.dart b/frontend/appflowy_flutter/lib/util/google_font_family_extension.dart index d20a42fc87..30a3229085 100644 --- a/frontend/appflowy_flutter/lib/util/google_font_family_extension.dart +++ b/frontend/appflowy_flutter/lib/util/google_font_family_extension.dart @@ -1,7 +1,6 @@ +import 'package:appflowy/shared/patterns/common_patterns.dart'; + extension GoogleFontsParser on String { - String parseFontFamilyName() { - final camelCase = RegExp('(?<=[a-z])[A-Z]'); - return replaceAll('_regular', '') - .replaceAllMapped(camelCase, (m) => ' ${m.group(0)}'); - } + String parseFontFamilyName() => replaceAll('_regular', '') + .replaceAllMapped(camelCaseRegex, (m) => ' ${m.group(0)}'); } diff --git a/frontend/appflowy_flutter/lib/util/string_extension.dart b/frontend/appflowy_flutter/lib/util/string_extension.dart index 0730a9f76d..3db25f7885 100644 --- a/frontend/appflowy_flutter/lib/util/string_extension.dart +++ b/frontend/appflowy_flutter/lib/util/string_extension.dart @@ -2,6 +2,8 @@ import 'dart:io'; import 'package:flutter/material.dart'; +import 'package:appflowy/shared/patterns/common_patterns.dart'; + extension StringExtension on String { static const _specialCharacters = r'\/:*?"<>| '; @@ -31,8 +33,6 @@ extension StringExtension on String { return null; } - /// Returns if the string is a appflowy cloud url. - bool get isAppFlowyCloudUrl { - return RegExp(r'^(https:\/\/)(.*)(\.appflowy\.cloud\/)(.*)').hasMatch(this); - } + /// Returns true if the string is a appflowy cloud url. + bool get isAppFlowyCloudUrl => appflowyCloudUrlRegex.hasMatch(this); } diff --git a/frontend/appflowy_flutter/lib/workspace/application/settings/application_data_storage.dart b/frontend/appflowy_flutter/lib/workspace/application/settings/application_data_storage.dart index 99a3b77d58..b0c8cb0948 100644 --- a/frontend/appflowy_flutter/lib/workspace/application/settings/application_data_storage.dart +++ b/frontend/appflowy_flutter/lib/workspace/application/settings/application_data_storage.dart @@ -1,10 +1,12 @@ import 'dart:io'; +import 'package:flutter/foundation.dart'; + import 'package:appflowy/core/config/kv.dart'; import 'package:appflowy/core/config/kv_keys.dart'; +import 'package:appflowy/shared/patterns/common_patterns.dart'; import 'package:appflowy/startup/startup.dart'; import 'package:appflowy_backend/log.dart'; -import 'package:flutter/foundation.dart'; import 'package:path/path.dart' as p; import '../../../startup/tasks/prelude.dart'; @@ -26,7 +28,7 @@ class ApplicationDataStorage { if (Platform.isMacOS) { // remove the prefix `/Volumes/*` - path = path.replaceFirst(RegExp('^/Volumes/[^/]+'), ''); + path = path.replaceFirst(macOSVolumesRegex, ''); } else if (Platform.isWindows) { path = path.replaceAll('/', '\\'); } diff --git a/frontend/appflowy_flutter/lib/workspace/application/settings/date_time/time_patterns.dart b/frontend/appflowy_flutter/lib/workspace/application/settings/date_time/time_patterns.dart deleted file mode 100644 index 57443b23f8..0000000000 --- a/frontend/appflowy_flutter/lib/workspace/application/settings/date_time/time_patterns.dart +++ /dev/null @@ -1,18 +0,0 @@ -/// RegExp to match Twelve Hour formats -/// Source: https://stackoverflow.com/a/33906224 -/// -/// Matches eg: "05:05 PM", "5:50 Pm", "10:59 am", etc. -/// -final _twelveHourTimePattern = - RegExp(r'\b((1[0-2]|0?[1-9]):([0-5][0-9]) ([AaPp][Mm]))'); -bool isTwelveHourTime(String? time) => - _twelveHourTimePattern.hasMatch(time ?? ''); - -/// RegExp to match Twenty Four Hour formats -/// Source: https://stackoverflow.com/a/7536768 -/// -/// Matches eg: "0:01", "04:59", "16:30", etc. -/// -final _twentyFourHourtimePattern = RegExp(r'^([0-1]?[0-9]|2[0-3]):[0-5][0-9]$'); -bool isTwentyFourHourTime(String? time) => - _twentyFourHourtimePattern.hasMatch(time ?? ''); diff --git a/frontend/appflowy_flutter/lib/workspace/presentation/settings/widgets/settings_appearance/font_family_setting.dart b/frontend/appflowy_flutter/lib/workspace/presentation/settings/widgets/settings_appearance/font_family_setting.dart index 922316fcbe..7a25e2f032 100644 --- a/frontend/appflowy_flutter/lib/workspace/presentation/settings/widgets/settings_appearance/font_family_setting.dart +++ b/frontend/appflowy_flutter/lib/workspace/presentation/settings/widgets/settings_appearance/font_family_setting.dart @@ -95,7 +95,7 @@ class _FontFamilyDropDownState extends State { return FlowySettingValueDropDown( popoverKey: ThemeFontFamilySetting.popoverKey, popoverController: widget.popoverController, - currentValue: parseFontFamilyName(widget.currentFontFamily), + currentValue: widget.currentFontFamily.parseFontFamilyName(), onClose: () { query.value = ''; widget.onClose?.call(); @@ -162,18 +162,11 @@ class _FontFamilyDropDownState extends State { ); } - String parseFontFamilyName(String fontFamilyName) { - final camelCase = RegExp('(?<=[a-z])[A-Z]'); - return fontFamilyName - .replaceAll('_regular', '') - .replaceAllMapped(camelCase, (m) => ' ${m.group(0)}'); - } - Widget _fontFamilyItemButton( BuildContext context, TextStyle style, ) { - final buttonFontFamily = parseFontFamilyName(style.fontFamily!); + final buttonFontFamily = style.fontFamily!.parseFontFamilyName(); return Tooltip( message: buttonFontFamily, @@ -184,21 +177,19 @@ class _FontFamilyDropDownState extends State { child: FlowyButton( onHover: (_) => FocusScope.of(context).unfocus(), text: FlowyText.medium( - parseFontFamilyName(style.fontFamily!), + buttonFontFamily, fontFamily: style.fontFamily!, ), rightIcon: - buttonFontFamily == parseFontFamilyName(widget.currentFontFamily) - ? const FlowySvg( - FlowySvgs.check_s, - ) + buttonFontFamily == widget.currentFontFamily.parseFontFamilyName() + ? const FlowySvg(FlowySvgs.check_s) : null, onTap: () { if (widget.onFontFamilyChanged != null) { widget.onFontFamilyChanged!(style.fontFamily!); } else { final fontFamily = style.fontFamily!.parseFontFamilyName(); - if (parseFontFamilyName(widget.currentFontFamily) != + if (widget.currentFontFamily.parseFontFamilyName() != buttonFontFamily) { context .read()