fleather icon indicating copy to clipboard operation
fleather copied to clipboard

Editable manages the viewport display

Open amantoux opened this issue 9 months ago • 1 comments

This PR is an attempt to mimic Flutter's Editable that behaves as a viewport (invisible elements are clipped out). The benefit here is that it eases future optimizations that would prevent from rendering element that are not visible. It also allows us to avoid the need to define a custom SingleChildScrollView

This is a first working example (some features are still missing)

@Amir-P let me know your thoughts (before I move on 😀 )

amantoux avatar Apr 30 '24 17:04 amantoux

  • [x] All tests pass
  • [x] Support no scroll
  • [x] SingleChildScrollView is removed

amantoux avatar May 09 '24 18:05 amantoux

Codecov Report

Attention: Patch coverage is 91.66667% with 10 lines in your changes missing coverage. Please review.

Project coverage is 87.97%. Comparing base (178d2bb) to head (87d534f). Report is 5 commits behind head on master.

Files Patch % Lines
packages/fleather/lib/src/widgets/editor.dart 90.32% 6 Missing :warning:
packages/fleather/lib/src/rendering/editor.dart 93.61% 3 Missing :warning:
...kages/fleather/lib/src/widgets/text_selection.dart 90.90% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #338      +/-   ##
==========================================
+ Coverage   87.90%   87.97%   +0.07%     
==========================================
  Files          64       61       -3     
  Lines       10382    11105     +723     
==========================================
+ Hits         9126     9770     +644     
- Misses       1256     1335      +79     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jun 20 '24 08:06 codecov[bot]

It works mostly fine but I'm facing problem when using the editor or field in a scrolling view without limiting its height. Here is an example which works fine with master

Thanks @Amir-P for the example. As I see it the previous behavior was incorrect. By default, FleatherEditor is built with scrollable set to true. It should attempt to expand as much as possible (it seems to be the default behavior with scrollable in general, isn't it?). To achieve the desired behavior, one should pass scrollable: false to the constructor. The problem is the presence of an assert that compels scrollController to be non null when scrollable = false

All in all I propose to remove the assert and from now, to implement the desired behavior, you should pass scrollable: false.

WDYT?

amantoux avatar Jun 24 '24 16:06 amantoux

@amantoux TBH I didn't check the code at that time. Your suggestion sounds reasonable to me. Anyway, I found another problem that might be good to look into. It's possible to use a multiline TextField inside of a scrollable without any problem and it manages to bring into view the selected part of text by manipulating the parent scrolling view position. Doing the same with FleatherEditor or FleatherField results in BoxConstraints forces an infinite height error and with scrollable: false although laid out correctly, fails to bring into view the selected part of text. As far as I know the TextField uses a Scrollable in both multiline and non-multiline mode.

import 'dart:convert';
import 'dart:io';

import 'package:fleather/fleather.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';

void main() {
  runApp(const FleatherApp());
}

class FleatherApp extends StatelessWidget {
  const FleatherApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) => MaterialApp(
        debugShowCheckedModeBanner: false,
        theme: ThemeData.light(),
        darkTheme: ThemeData.dark(),
        title: 'Fleather - rich-text editor for Flutter',
        home: HomePage(),
      );
}

class HomePage extends StatefulWidget {
  const HomePage({Key? key}) : super(key: key);

  @override
  _HomePageState createState() => _HomePageState();
}

class _HomePageState extends State<HomePage> {
  final FocusNode _focusNode = FocusNode();
  FleatherController? _controller;

  @override
  void initState() {
    super.initState();
    if (kIsWeb) BrowserContextMenu.disableContextMenu();
    _initController();
  }

  @override
  void dispose() {
    super.dispose();
    if (kIsWeb) BrowserContextMenu.enableContextMenu();
  }

  Future<void> _initController() async {
    try {
      final result = await rootBundle.loadString('assets/welcome.json');
      final doc = ParchmentDocument.fromJson(jsonDecode(result));
      _controller = FleatherController(document: doc);
    } catch (err, st) {
      print('Cannot read welcome.json: $err\n$st');
      _controller = FleatherController();
    }
    setState(() {});
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(elevation: 0, title: Text('Fleather Demo')),
      body: _controller == null
          ? Center(child: const CircularProgressIndicator())
          : SingleChildScrollView(
              child: Column(
                children: [
                  FleatherToolbar.basic(controller: _controller!),
                  Divider(height: 1, thickness: 1, color: Colors.grey.shade200),
                  // FleatherEditor(
                  //   controller: _controller!,
                  //   focusNode: _focusNode,
                  //   padding: EdgeInsets.only(
                  //     left: 16,
                  //     right: 16,
                  //     bottom: MediaQuery.of(context).padding.bottom,
                  //   ),
                  //   onLaunchUrl: _launchUrl,
                  //   maxContentWidth: 800,
                  //   embedBuilder: _embedBuilder,
                  //   spellCheckConfiguration: SpellCheckConfiguration(
                  //       spellCheckService: DefaultSpellCheckService(),
                  //       misspelledSelectionColor: Colors.red,
                  //       misspelledTextStyle:
                  //           DefaultTextStyle.of(context).style),
                  // ),
                  TextField(
                    controller: TextEditingController(
                        text: _controller!.document.toPlainText()),
                    maxLines: null,
                  ),
                ],
              ),
            ),
    );
  }

  Widget _embedBuilder(BuildContext context, EmbedNode node) {
    if (node.value.type == 'icon') {
      final data = node.value.data;
      // Icons.rocket_launch_outlined
      return Icon(
        IconData(int.parse(data['codePoint']), fontFamily: data['fontFamily']),
        color: Color(int.parse(data['color'])),
        size: 18,
      );
    }

    if (node.value.type == 'image') {
      final sourceType = node.value.data['source_type'];
      ImageProvider? image;
      if (sourceType == 'assets') {
        image = AssetImage(node.value.data['source']);
      } else if (sourceType == 'file') {
        image = FileImage(File(node.value.data['source']));
      } else if (sourceType == 'url') {
        image = NetworkImage(node.value.data['source']);
      }
      if (image != null) {
        return Padding(
          // Caret takes 2 pixels, hence not symmetric padding values.
          padding: const EdgeInsets.only(left: 4, right: 2, top: 2, bottom: 2),
          child: Container(
            width: 300,
            height: 300,
            decoration: BoxDecoration(
              image: DecorationImage(image: image, fit: BoxFit.cover),
            ),
          ),
        );
      }
    }

    return defaultFleatherEmbedBuilder(context, node);
  }
}

Amir-P avatar Jun 25 '24 19:06 Amir-P

@amantoux TBH I didn't check the code at that time. Your suggestion sounds reasonable to me. Anyway, I found another problem that might be good to look into. It's possible to use a multiline TextField inside of a scrollable without any problem and it manages to bring into view the selected part of text by manipulating the parent scrolling view position. Doing the same with FleatherEditor or FleatherField results in BoxConstraints forces an infinite height error and with scrollable: false although laid out correctly, fails to bring into view the selected part of text. As far as I know the TextField uses a Scrollable in both multiline and non-multiline mode.

@Amir-P Here is the behavior I propose: If scrollable is true:

  • if expand is true, the editor fills all the space available in parent (therefore will cause an assert error if place in a Column or a Scrollable). If text inside is greater than the available space, the scroll is effective.
  • if expand is false, the editor behave as a shrink wrap ListView: it takes only the required space but can grow indefinitely as text is being added to the document. If text inside overflows the available space, the scroll is effective.

If scrollable is false:

  • if expand is true, the editor takes all space available in parent. If text inside is greater than the available space, it is clipped.
  • if expand is false, the editor takes only required space, if the latter is bigger that the available space in parent, text will be clipped if greater than available space

WDYT?

amantoux avatar Jul 07 '24 15:07 amantoux

Sorry Alan I was busy. Will try to get back to you today. @amantoux

Amir-P avatar Jul 10 '24 13:07 Amir-P

The behavior you've described sounds good to me, Alan. Do you need to make changes? @amantoux

Amir-P avatar Jul 13 '24 19:07 Amir-P

The behavior you've described sounds good to me, Alan. Do you need to make changes? @amantoux

@Amir-P Yes I pushed the changes

amantoux avatar Jul 14 '24 08:07 amantoux

@amantoux TBH I didn't check the code at that time. Your suggestion sounds reasonable to me. Anyway, I found another problem that might be good to look into. It's possible to use a multiline TextField inside of a scrollable without any problem and it manages to bring into view the selected part of text by manipulating the parent scrolling view position. Doing the same with FleatherEditor or FleatherField results in BoxConstraints forces an infinite height error and with scrollable: false although laid out correctly, fails to bring into view the selected part of text. As far as I know the TextField uses a Scrollable in both multiline and non-multiline mode.

import 'dart:convert';
import 'dart:io';

import 'package:fleather/fleather.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';

void main() {
  runApp(const FleatherApp());
}

class FleatherApp extends StatelessWidget {
  const FleatherApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) => MaterialApp(
        debugShowCheckedModeBanner: false,
        theme: ThemeData.light(),
        darkTheme: ThemeData.dark(),
        title: 'Fleather - rich-text editor for Flutter',
        home: HomePage(),
      );
}

class HomePage extends StatefulWidget {
  const HomePage({Key? key}) : super(key: key);

  @override
  _HomePageState createState() => _HomePageState();
}

class _HomePageState extends State<HomePage> {
  final FocusNode _focusNode = FocusNode();
  FleatherController? _controller;

  @override
  void initState() {
    super.initState();
    if (kIsWeb) BrowserContextMenu.disableContextMenu();
    _initController();
  }

  @override
  void dispose() {
    super.dispose();
    if (kIsWeb) BrowserContextMenu.enableContextMenu();
  }

  Future<void> _initController() async {
    try {
      final result = await rootBundle.loadString('assets/welcome.json');
      final doc = ParchmentDocument.fromJson(jsonDecode(result));
      _controller = FleatherController(document: doc);
    } catch (err, st) {
      print('Cannot read welcome.json: $err\n$st');
      _controller = FleatherController();
    }
    setState(() {});
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(elevation: 0, title: Text('Fleather Demo')),
      body: _controller == null
          ? Center(child: const CircularProgressIndicator())
          : SingleChildScrollView(
              child: Column(
                children: [
                  FleatherToolbar.basic(controller: _controller!),
                  Divider(height: 1, thickness: 1, color: Colors.grey.shade200),
                  // FleatherEditor(
                  //   controller: _controller!,
                  //   focusNode: _focusNode,
                  //   padding: EdgeInsets.only(
                  //     left: 16,
                  //     right: 16,
                  //     bottom: MediaQuery.of(context).padding.bottom,
                  //   ),
                  //   onLaunchUrl: _launchUrl,
                  //   maxContentWidth: 800,
                  //   embedBuilder: _embedBuilder,
                  //   spellCheckConfiguration: SpellCheckConfiguration(
                  //       spellCheckService: DefaultSpellCheckService(),
                  //       misspelledSelectionColor: Colors.red,
                  //       misspelledTextStyle:
                  //           DefaultTextStyle.of(context).style),
                  // ),
                  TextField(
                    controller: TextEditingController(
                        text: _controller!.document.toPlainText()),
                    maxLines: null,
                  ),
                ],
              ),
            ),
    );
  }

  Widget _embedBuilder(BuildContext context, EmbedNode node) {
    if (node.value.type == 'icon') {
      final data = node.value.data;
      // Icons.rocket_launch_outlined
      return Icon(
        IconData(int.parse(data['codePoint']), fontFamily: data['fontFamily']),
        color: Color(int.parse(data['color'])),
        size: 18,
      );
    }

    if (node.value.type == 'image') {
      final sourceType = node.value.data['source_type'];
      ImageProvider? image;
      if (sourceType == 'assets') {
        image = AssetImage(node.value.data['source']);
      } else if (sourceType == 'file') {
        image = FileImage(File(node.value.data['source']));
      } else if (sourceType == 'url') {
        image = NetworkImage(node.value.data['source']);
      }
      if (image != null) {
        return Padding(
          // Caret takes 2 pixels, hence not symmetric padding values.
          padding: const EdgeInsets.only(left: 4, right: 2, top: 2, bottom: 2),
          child: Container(
            width: 300,
            height: 300,
            decoration: BoxDecoration(
              image: DecorationImage(image: image, fit: BoxFit.cover),
            ),
          ),
        );
      }
    }

    return defaultFleatherEmbedBuilder(context, node);
  }
}

Hi, I've been having an issue and it seems to relate to the behaviour listed above. I'm trying to use FleatherField like I would a normal TextFormField, but the cursor always ends up underneath the keyboard. Other than this one issue I'm loving Fleather, but this is unfortunately blocking me, as we have multiple editable fields in a scrollable list, so they need to respect keyboard correctly.

Reading the above I hoped the issue was resolved but doesn't seem to be from my testing. @Amir-P was the issue you found with scrolling fixed as part of this PR?

kane-knowby avatar Sep 03 '24 08:09 kane-knowby

Hey @kane-knowby, Thanks for bringing this to our attention. The issue you are referring to wasn't fixed with this PR since I realized that we had the issue even before this PR. But it's something that we need to address. Can you please create an issue for it?

Amir-P avatar Sep 04 '24 11:09 Amir-P