[Bug] Unclear scroll assertion

[saif-ellafi]

Bug Description

Dear Appflowy team,

is this assertion really necessary? scrollable_helpers.dart has this snippet here:

    assert(
      globalRect.size.width >= _dragTargetRelatedToScrollOrigin.size.width &&
          globalRect.size.height >=
              _dragTargetRelatedToScrollOrigin.size.height,
      'Drag target size is larger than scrollable size, which may cause bouncing',
    );

This assertion is giving me tons of errors in my AppflowyEditor running in a popup widget, with custom layout and height, and honestly, it works perfectly fine without this assertion. With it, I get errors that make the editor unusable.

I tried many different things but can't figure out which assertion am I really failing and why is this assertion mandatory. I have double or even triple scroll areas in my current Widget and I think something is not accurate here. Any thoughts or ideas? I am manually setting the height of the Popup and calculating the height of the editor so it falls comfortable for writing in mobile phones.

return _conditionallyExpanded(
  child: Stack(
    children: [
      Container(
        padding: EdgeInsets.symmetric(horizontal: 5.0),
        decoration: BoxDecoration(
          border: Border.all(
            width: 1.5,  
            color: PUMCompanion.gameTheme.tertiaryTextStyle.color!,
          ),
          borderRadius: PUMCompanion.gameTheme.borderRadius,
        ),
        child: PopScope(
          onPopInvokedWithResult: (didPop, popResult) {
            if (didPop) {
              widget.doSaveGame();
            }
          },
          child: Padding(
            padding: const EdgeInsets.symmetric(vertical: 10.0),
            child: RichEditorDesktop(
              editorState: _textControllers[index][descIndex],
              index: index,
              descIndex: descIndex,
              widget: widget,
              tabName: _tabs[index],
            )
          ),
        ),
      ),

where

return Column(
  children: [
    SizedBox(
      height: 16,
      child: _FixedToolbar(
        editorState: widget.editorState,
      ),
    ),
    Divider(color: PUMCompanion.gameTheme.borderColor),
    Expanded(
      child: AppFlowyEditor(
        editorState: widget.editorState,
        editorScrollController: _editorScrollController,
        focusNode: _focusNode,
        autoFocus: false,
        disableAutoScroll: false,
        editorStyle: EditorStyle(

Cheers and thanks again for the amazing plugin

How to Reproduce

Just load the AppFloyEditor in a context of a LayoutBuilder with custom height constraints and vertical scroll areas.

Expected Behavior

Do not assert if this assertion is not mandatory.

Operating System

All systems

AppFlowy Editor Version(s)

main

Screenshots

No response

Additional Context

No response

[saif-ellafi]

I think I may understand what is this about, but honestly no idea how to fix it.

The cursor is going overflow beyond the rich editor

[LucasXu0]

@saif-ellafi I think this assertion is not necessary, but it might be triggered by the inccorect size of the widget.

[saif-ellafi]

Yes, all the issues I am sharing are by placing the editor in a big popup dialog.

This one in particular I can close, I have disabled the assertion in my Fork because I needed it for a while, but now I figured out to expand the Editor properly and this did not come again yet, I was I think setting the height of the editor in the Dialog manually. For that particular case the Assertion was "annoying" with not much of a bad side effect, maybe convert it to a warning instead?

Thank you for taking a look into it.

[mikefaust-jm]

@saif-ellafi I just encountered the same problem. How did you solve it? I tried to set a height with a container around the editor without success. The auto_expand_editor example here throws the same error. Maybe we can fix it for other people with a similar use case.

[saif-ellafi]

Indeed I don't recall now other than removing any specification of height, in favor of Expanded widget and shrink_wrap properties in any parents. Will double check my code and come back if I added anything else.

As I was saying earlier, personally I find the assertion too strong, I survived long in production with a fork with the assertion removed without issues, but I'm honestly not sure I fully understand what the assertion is protecting us from.

[mikefaust-jm]

I found the fix to the assertion error. The default autoScrollEdgeOffset value of 220 is too high for such a small field. Setting it to a much lower value of around 24 removes the error.