From e366d83191d7e8dce319a9162462a06c801e4c20 Mon Sep 17 00:00:00 2001 From: Talha Date: Tue, 24 Sep 2024 14:27:04 +0100 Subject: [PATCH 1/3] fix html editor not responding on empty description content --- app/lib/common/utils/utils.dart | 28 +++++++++++++++++++ .../widgets/edit_html_description_sheet.dart | 11 ++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/app/lib/common/utils/utils.dart b/app/lib/common/utils/utils.dart index f1d79bebb8dc..33c1d703ed94 100644 --- a/app/lib/common/utils/utils.dart +++ b/app/lib/common/utils/utils.dart @@ -15,6 +15,8 @@ import 'package:flutter_easyloading/flutter_easyloading.dart'; import 'package:flutter_gen/gen_l10n/l10n.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; +import 'package:html/dom.dart'; +import 'package:html/parser.dart'; import 'package:intl/intl.dart'; import 'package:jiffy/jiffy.dart'; import 'package:logging/logging.dart'; @@ -458,3 +460,29 @@ extension Let on T? { return value == null ? null : op(value); } } + +// check the input string if its valid HTML +bool isValidHtml(String? html) { + if (html == null || html.isEmpty) { + return false; + } + + try { + // Parse the HTML + Document document = parse(html); + + // Check if there are any elements in the body + var body = document.body; + if (body == null || body.nodes.isEmpty) { + return false; + } + + // Check if the body contains only whitespace + bool hasNonWhitespace = body.text.trim().isNotEmpty; + + return hasNonWhitespace; + } catch (e) { + // If parsing fails, it's not valid HTML + return false; + } +} diff --git a/app/lib/common/widgets/edit_html_description_sheet.dart b/app/lib/common/widgets/edit_html_description_sheet.dart index 7dac686240e2..e1cddbc52365 100644 --- a/app/lib/common/widgets/edit_html_description_sheet.dart +++ b/app/lib/common/widgets/edit_html_description_sheet.dart @@ -1,5 +1,6 @@ import 'package:acter/common/themes/colors/color_scheme.dart'; import 'package:acter/common/toolkit/buttons/primary_action_button.dart'; +import 'package:acter/common/utils/utils.dart'; import 'package:acter/common/widgets/html_editor.dart'; import 'package:appflowy_editor/appflowy_editor.dart'; import 'package:flutter/material.dart'; @@ -58,8 +59,8 @@ class _EditHtmlDescriptionSheetState super.initState(); final html = widget.descriptionHtmlValue; final markdown = widget.descriptionMarkdownValue ?? ''; - final document = html != null - ? ActerDocumentHelpers.fromHtml(html) + final document = isValidHtml(html) + ? ActerDocumentHelpers.fromHtml(html!) : ActerDocumentHelpers.fromMarkdown(markdown); textEditorState = EditorState(document: document); } @@ -97,13 +98,17 @@ class _EditHtmlDescriptionSheetState ActerPrimaryActionButton( onPressed: () { // No need to change - final htmlBodyDescription = textEditorState.intoHtml(); + String htmlBodyDescription = textEditorState.intoHtml(); + htmlBodyDescription = isValidHtml(htmlBodyDescription) + ? htmlBodyDescription + : ''; final plainDescription = textEditorState.intoMarkdown(); if (htmlBodyDescription == widget.descriptionHtmlValue || plainDescription == widget.descriptionMarkdownValue) { Navigator.pop(context); return; } + widget.onSave(htmlBodyDescription, plainDescription); }, child: Text(L10n.of(context).save), From 404f46afa35e546ebbdbdc5cd2f64b4f7d61e7ac Mon Sep 17 00:00:00 2001 From: Talha Date: Tue, 24 Sep 2024 18:10:11 +0100 Subject: [PATCH 2/3] Feedback review: reduce parse function and check validity in helper fn --- app/lib/common/utils/utils.dart | 28 ------------------- .../widgets/edit_html_description_sheet.dart | 14 +++++----- app/lib/common/widgets/html_editor.dart | 14 ++++++++-- .../comments/widgets/create_comment.dart | 6 +++- .../events/pages/create_event_page.dart | 13 +++++++-- .../features/news/pages/add_news_page.dart | 9 ++++-- .../tasks/sheets/create_update_task_list.dart | 6 +++- 7 files changed, 45 insertions(+), 45 deletions(-) diff --git a/app/lib/common/utils/utils.dart b/app/lib/common/utils/utils.dart index 33c1d703ed94..f1d79bebb8dc 100644 --- a/app/lib/common/utils/utils.dart +++ b/app/lib/common/utils/utils.dart @@ -15,8 +15,6 @@ import 'package:flutter_easyloading/flutter_easyloading.dart'; import 'package:flutter_gen/gen_l10n/l10n.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; -import 'package:html/dom.dart'; -import 'package:html/parser.dart'; import 'package:intl/intl.dart'; import 'package:jiffy/jiffy.dart'; import 'package:logging/logging.dart'; @@ -460,29 +458,3 @@ extension Let on T? { return value == null ? null : op(value); } } - -// check the input string if its valid HTML -bool isValidHtml(String? html) { - if (html == null || html.isEmpty) { - return false; - } - - try { - // Parse the HTML - Document document = parse(html); - - // Check if there are any elements in the body - var body = document.body; - if (body == null || body.nodes.isEmpty) { - return false; - } - - // Check if the body contains only whitespace - bool hasNonWhitespace = body.text.trim().isNotEmpty; - - return hasNonWhitespace; - } catch (e) { - // If parsing fails, it's not valid HTML - return false; - } -} diff --git a/app/lib/common/widgets/edit_html_description_sheet.dart b/app/lib/common/widgets/edit_html_description_sheet.dart index e1cddbc52365..1442602fb06c 100644 --- a/app/lib/common/widgets/edit_html_description_sheet.dart +++ b/app/lib/common/widgets/edit_html_description_sheet.dart @@ -1,6 +1,5 @@ import 'package:acter/common/themes/colors/color_scheme.dart'; import 'package:acter/common/toolkit/buttons/primary_action_button.dart'; -import 'package:acter/common/utils/utils.dart'; import 'package:acter/common/widgets/html_editor.dart'; import 'package:appflowy_editor/appflowy_editor.dart'; import 'package:flutter/material.dart'; @@ -59,10 +58,14 @@ class _EditHtmlDescriptionSheetState super.initState(); final html = widget.descriptionHtmlValue; final markdown = widget.descriptionMarkdownValue ?? ''; - final document = isValidHtml(html) - ? ActerDocumentHelpers.fromHtml(html!) + final document = html != null + ? ActerDocumentHelpers.fromHtml(html) : ActerDocumentHelpers.fromMarkdown(markdown); - textEditorState = EditorState(document: document); + if (document != null) { + textEditorState = EditorState(document: document); + } else { + textEditorState = EditorState.blank(); + } } @override @@ -99,9 +102,6 @@ class _EditHtmlDescriptionSheetState onPressed: () { // No need to change String htmlBodyDescription = textEditorState.intoHtml(); - htmlBodyDescription = isValidHtml(htmlBodyDescription) - ? htmlBodyDescription - : ''; final plainDescription = textEditorState.intoMarkdown(); if (htmlBodyDescription == widget.descriptionHtmlValue || plainDescription == widget.descriptionMarkdownValue) { diff --git a/app/lib/common/widgets/html_editor.dart b/app/lib/common/widgets/html_editor.dart index bdf050df13f2..5f6f0d8e1c07 100644 --- a/app/lib/common/widgets/html_editor.dart +++ b/app/lib/common/widgets/html_editor.dart @@ -50,11 +50,19 @@ extension ActerEditorStateHelpers on EditorState { } extension ActerDocumentHelpers on Document { - static Document fromHtml( + static Document? fromHtml( String content, { AppFlowyEditorHTMLCodec? codec, }) { - return (codec ?? defaultHtmlCodec).decode(content); + if (content.isEmpty) { + return null; + } + + Document document = (codec ?? defaultHtmlCodec).decode(content); + if (document.isEmpty) { + return null; + } + return document; } static Document fromMarkdown( @@ -67,7 +75,7 @@ extension ActerDocumentHelpers on Document { static Document fromMsgContent(MsgContent msgContent) { final formattedBody = msgContent.formattedBody(); if (formattedBody != null) { - return ActerDocumentHelpers.fromHtml(formattedBody); + return ActerDocumentHelpers.fromHtml(formattedBody)!; } else { return ActerDocumentHelpers.fromMarkdown(msgContent.body()); } diff --git a/app/lib/features/comments/widgets/create_comment.dart b/app/lib/features/comments/widgets/create_comment.dart index e7396eeb1a9e..06d750818c13 100644 --- a/app/lib/features/comments/widgets/create_comment.dart +++ b/app/lib/features/comments/widgets/create_comment.dart @@ -62,7 +62,11 @@ class _CreateCommentWidgetState extends ConsumerState { final document = html != null ? ActerDocumentHelpers.fromHtml(html) : ActerDocumentHelpers.fromMarkdown(body); - textEditorState = EditorState(document: document); + if (document != null) { + textEditorState = EditorState(document: document); + } else { + textEditorState = EditorState.blank(); + } }, ), ), diff --git a/app/lib/features/events/pages/create_event_page.dart b/app/lib/features/events/pages/create_event_page.dart index eb199d88369e..aead5fd22878 100644 --- a/app/lib/features/events/pages/create_event_page.dart +++ b/app/lib/features/events/pages/create_event_page.dart @@ -58,8 +58,11 @@ class CreateEventPageConsumerState extends ConsumerState { if (desc != null) { final formatted = desc.formatted(); if (formatted != null) { - textEditorState = - EditorState(document: ActerDocumentHelpers.fromHtml(formatted)); + final document = ActerDocumentHelpers.fromHtml(formatted); + if (document != null) { + textEditorState = EditorState(document: document); + } + textEditorState = EditorState.blank(); } else { textEditorState = EditorState( document: ActerDocumentHelpers.fromMarkdown( @@ -380,7 +383,11 @@ class CreateEventPageConsumerState extends ConsumerState { final document = html != null ? ActerDocumentHelpers.fromHtml(html) : ActerDocumentHelpers.fromMarkdown(body); - textEditorState = EditorState(document: document); + if (document != null) { + textEditorState = EditorState(document: document); + } else { + textEditorState = EditorState.blank(); + } }, ), ), diff --git a/app/lib/features/news/pages/add_news_page.dart b/app/lib/features/news/pages/add_news_page.dart index dcc354ee2700..a71d8ba214a6 100644 --- a/app/lib/features/news/pages/add_news_page.dart +++ b/app/lib/features/news/pages/add_news_page.dart @@ -56,13 +56,18 @@ class AddNewsState extends ConsumerState { final next = nextState.currentNewsSlide!; final document = next.html != null ? ActerDocumentHelpers.fromHtml(next.html!) - : ActerDocumentHelpers.fromMarkdown(next.text ?? ''); + : ActerDocumentHelpers.fromMarkdown(next.text!); + final autoFocus = (next.html?.isEmpty ?? true) && (next.text?.isEmpty ?? true); setState(() { selectedNewsPost = next; - textEditorState = EditorState(document: document); + if (document != null) { + textEditorState = EditorState(document: document); + } else { + textEditorState = EditorState.blank(); + } }); if (autoFocus) { diff --git a/app/lib/features/tasks/sheets/create_update_task_list.dart b/app/lib/features/tasks/sheets/create_update_task_list.dart index 57d54f3424a1..de576cad26d2 100644 --- a/app/lib/features/tasks/sheets/create_update_task_list.dart +++ b/app/lib/features/tasks/sheets/create_update_task_list.dart @@ -173,7 +173,11 @@ class _CreateUpdateTaskListConsumerState final document = html != null ? ActerDocumentHelpers.fromHtml(html) : ActerDocumentHelpers.fromMarkdown(body); - textEditorState = EditorState(document: document); + if (document != null) { + textEditorState = EditorState(document: document); + } else { + textEditorState = EditorState.blank(); + } }, ), ), From a88c552f12db7539b1dc0237fdaee25bfc37c841 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Wed, 25 Sep 2024 16:04:08 +0100 Subject: [PATCH 3/3] Refactor DocumentHelper to be less error-prone --- .../widgets/edit_html_description_sheet.dart | 16 ++++----- app/lib/common/widgets/html_editor.dart | 29 ++++++++++----- .../comments/widgets/create_comment.dart | 14 ++++---- .../events/pages/create_event_page.dart | 36 ++++++++----------- .../features/news/pages/add_news_page.dart | 11 ++---- .../tasks/sheets/create_update_task_list.dart | 14 ++++---- 6 files changed, 56 insertions(+), 64 deletions(-) diff --git a/app/lib/common/widgets/edit_html_description_sheet.dart b/app/lib/common/widgets/edit_html_description_sheet.dart index 1442602fb06c..af62b2c2415e 100644 --- a/app/lib/common/widgets/edit_html_description_sheet.dart +++ b/app/lib/common/widgets/edit_html_description_sheet.dart @@ -56,16 +56,12 @@ class _EditHtmlDescriptionSheetState @override void initState() { super.initState(); - final html = widget.descriptionHtmlValue; - final markdown = widget.descriptionMarkdownValue ?? ''; - final document = html != null - ? ActerDocumentHelpers.fromHtml(html) - : ActerDocumentHelpers.fromMarkdown(markdown); - if (document != null) { - textEditorState = EditorState(document: document); - } else { - textEditorState = EditorState.blank(); - } + textEditorState = EditorState( + document: ActerDocumentHelpers.parse( + widget.descriptionMarkdownValue ?? '', + htmlContent: widget.descriptionHtmlValue, + ), + ); } @override diff --git a/app/lib/common/widgets/html_editor.dart b/app/lib/common/widgets/html_editor.dart index 5f6f0d8e1c07..6b2ccd3783d4 100644 --- a/app/lib/common/widgets/html_editor.dart +++ b/app/lib/common/widgets/html_editor.dart @@ -50,7 +50,7 @@ extension ActerEditorStateHelpers on EditorState { } extension ActerDocumentHelpers on Document { - static Document? fromHtml( + static Document? _fromHtml( String content, { AppFlowyEditorHTMLCodec? codec, }) { @@ -65,20 +65,33 @@ extension ActerDocumentHelpers on Document { return document; } - static Document fromMarkdown( + static Document _fromMarkdown( String content, { AppFlowyEditorMarkdownCodec? codec, }) { return (codec ?? defaultMarkdownCodec).decode(content); } - static Document fromMsgContent(MsgContent msgContent) { - final formattedBody = msgContent.formattedBody(); - if (formattedBody != null) { - return ActerDocumentHelpers.fromHtml(formattedBody)!; - } else { - return ActerDocumentHelpers.fromMarkdown(msgContent.body()); + static Document parse( + String content, { + String? htmlContent, + AppFlowyEditorMarkdownCodec? codec, + }) { + if (htmlContent != null) { + final document = ActerDocumentHelpers._fromHtml(htmlContent); + if (document != null) { + return document; + } } + // fallback: parse from markdown + return ActerDocumentHelpers._fromMarkdown(content); + } + + static Document fromMsgContent(MsgContent msgContent) { + return ActerDocumentHelpers.parse( + msgContent.body(), + htmlContent: msgContent.formattedBody(), + ); } } diff --git a/app/lib/features/comments/widgets/create_comment.dart b/app/lib/features/comments/widgets/create_comment.dart index 06d750818c13..b7b505e5678e 100644 --- a/app/lib/features/comments/widgets/create_comment.dart +++ b/app/lib/features/comments/widgets/create_comment.dart @@ -59,14 +59,12 @@ class _CreateCommentWidgetState extends ConsumerState { editorState: textEditorState, footer: const SizedBox(), onChanged: (body, html) { - final document = html != null - ? ActerDocumentHelpers.fromHtml(html) - : ActerDocumentHelpers.fromMarkdown(body); - if (document != null) { - textEditorState = EditorState(document: document); - } else { - textEditorState = EditorState.blank(); - } + textEditorState = EditorState( + document: ActerDocumentHelpers.parse( + body, + htmlContent: html, + ), + ); }, ), ), diff --git a/app/lib/features/events/pages/create_event_page.dart b/app/lib/features/events/pages/create_event_page.dart index aead5fd22878..3e4d529d1881 100644 --- a/app/lib/features/events/pages/create_event_page.dart +++ b/app/lib/features/events/pages/create_event_page.dart @@ -56,20 +56,14 @@ class CreateEventPageConsumerState extends ConsumerState { // description final desc = event.description(); if (desc != null) { - final formatted = desc.formatted(); - if (formatted != null) { - final document = ActerDocumentHelpers.fromHtml(formatted); - if (document != null) { - textEditorState = EditorState(document: document); - } - textEditorState = EditorState.blank(); - } else { - textEditorState = EditorState( - document: ActerDocumentHelpers.fromMarkdown( - desc.body(), - ), - ); - } + textEditorState = EditorState( + document: ActerDocumentHelpers.parse( + desc.body(), + htmlContent: desc.formatted(), + ), + ); + } else { + textEditorState = EditorState.blank(); } // Getting start and end date time @@ -380,14 +374,12 @@ class CreateEventPageConsumerState extends ConsumerState { editable: true, autoFocus: false, onChanged: (body, html) { - final document = html != null - ? ActerDocumentHelpers.fromHtml(html) - : ActerDocumentHelpers.fromMarkdown(body); - if (document != null) { - textEditorState = EditorState(document: document); - } else { - textEditorState = EditorState.blank(); - } + textEditorState = EditorState( + document: ActerDocumentHelpers.parse( + body, + htmlContent: html, + ), + ); }, ), ), diff --git a/app/lib/features/news/pages/add_news_page.dart b/app/lib/features/news/pages/add_news_page.dart index a71d8ba214a6..732ca421f0ea 100644 --- a/app/lib/features/news/pages/add_news_page.dart +++ b/app/lib/features/news/pages/add_news_page.dart @@ -54,20 +54,15 @@ class AddNewsState extends ConsumerState { final changed = prevState?.currentNewsSlide != nextState.currentNewsSlide; if (isText && changed) { final next = nextState.currentNewsSlide!; - final document = next.html != null - ? ActerDocumentHelpers.fromHtml(next.html!) - : ActerDocumentHelpers.fromMarkdown(next.text!); + final document = + ActerDocumentHelpers.parse(next.text ?? '', htmlContent: next.html); final autoFocus = (next.html?.isEmpty ?? true) && (next.text?.isEmpty ?? true); setState(() { selectedNewsPost = next; - if (document != null) { - textEditorState = EditorState(document: document); - } else { - textEditorState = EditorState.blank(); - } + textEditorState = EditorState(document: document); }); if (autoFocus) { diff --git a/app/lib/features/tasks/sheets/create_update_task_list.dart b/app/lib/features/tasks/sheets/create_update_task_list.dart index de576cad26d2..b076e59d6122 100644 --- a/app/lib/features/tasks/sheets/create_update_task_list.dart +++ b/app/lib/features/tasks/sheets/create_update_task_list.dart @@ -170,14 +170,12 @@ class _CreateUpdateTaskListConsumerState editable: true, autoFocus: false, onChanged: (body, html) { - final document = html != null - ? ActerDocumentHelpers.fromHtml(html) - : ActerDocumentHelpers.fromMarkdown(body); - if (document != null) { - textEditorState = EditorState(document: document); - } else { - textEditorState = EditorState.blank(); - } + textEditorState = EditorState( + document: ActerDocumentHelpers.parse( + body, + htmlContent: html, + ), + ); }, ), ),