Added an option for ClipboardPaste button to disable ClipboardMonitor #2427

Maksim-Nikolaev · 2025-01-05T18:56:50Z

Description

According to the issue #2421 ClipboardMonitor was causing lag if "paste" button was enabled in QuillSimpleToolbar.

That was because of the periodic 1 second check for Clipboard content. In case with large strings or images in clipboard, it would lag out the application and slow down everything drastically.

class ClipboardMonitor {
  bool _canPaste = false;
  bool get canPaste => _canPaste;
  Timer? _timer;

  void monitorClipboard(bool add, void Function() listener) {
    if (kIsWeb) return;
    if (add) {
      _timer = Timer.periodic(
          const Duration(seconds: 1), (timer) => _update(listener));
    } else {
      _timer?.cancel();
    }
  }

  Future<void> _update(void Function() listener) async {
    final clipboardService = ClipboardServiceProvider.instance;
    if (await clipboardService.hasClipboardContent) {
      _canPaste = true;
      listener();
    }
  }
}

This is not a breaking change, however it will change the behavior of availability for "paste" button for QuillSimpleToolbar since it always be enabled (except of readonly mode).

Related Issues

Fix ClipboardMonitor causes background lag when enabling clipboard buttons #2421

Type of Change

🛠️ Bug fix: Resolves an issue without changing current behavior.

EchoEllet · 2025-01-05T19:06:26Z

Thanks for your contribution.

This is not a breaking change

A breaking change is not only the kind that requires users to update their code to compile, but to also keep the current state without any behavior breaking.

Changing the default of a value is also a breaking change.

What would happen if the user pressed the paste button and there was nothing to paste?

I suggest providing the option to override the canPaste state instead, so they don't need to depend on ClipboardMonitor.

I'm with the idea of disabling something that's causing issues. However, users should have the option to revert to the old behavior in case they still want ClipboardMonitor with a signle boolean parameter.

Maksim-Nikolaev · 2025-01-05T19:19:22Z

A breaking change is not the kind that requires users to update their code to compile, but to also keep the current state without any behavior breaking. Changing the default of a value is also a breaking change.

Ok I understand

What would happen if the user pressed the paste button and there was nothing to paste?

Nothing will happen. There are few checks that prevent breaking everything on calling "paste" function.

I double checked it with Web & Android version and I strongly believe that iOS will not break either (don't have it right now, so I can't check)

QuillController.clipboardPaste handles this quite well, here is the full function:

  /// Returns whether paste operation was handled here.
  /// [updateEditor] is called if paste operation was successful.
  @experimental
  Future<bool> clipboardPaste({void Function()? updateEditor}) async {
    if (readOnly || !selection.isValid) return true;

    final clipboardConfig = config.clipboardConfig;

    if (await clipboardConfig?.onClipboardPaste?.call() == true) {
      updateEditor?.call();
      return true;
    }

    final pasteInternalImageSuccess = await _pasteInternalImage();
    if (pasteInternalImageSuccess) {
      updateEditor?.call();
      return true;
    }

    const enableExternalRichPasteDefault = true;
    if (clipboardConfig?.enableExternalRichPaste ??
        enableExternalRichPasteDefault) {
      final pasteHtmlSuccess = await pasteHTML();
      if (pasteHtmlSuccess) {
        updateEditor?.call();
        return true;
      }

      final pasteMarkdownSuccess = await pasteMarkdown();
      if (pasteMarkdownSuccess) {
        updateEditor?.call();
        return true;
      }
    }

    final clipboardService = ClipboardServiceProvider.instance;

    final onImagePaste = clipboardConfig?.onImagePaste;
    if (onImagePaste != null) {
      final imageBytes = await clipboardService.getImageFile();

      if (imageBytes != null) {
        final imageUrl = await onImagePaste(imageBytes);
        if (imageUrl != null) {
          replaceText(
            plainTextEditingValue.selection.end,
            0,
            BlockEmbed.image(imageUrl),
            null,
          );
          updateEditor?.call();
          return true;
        }
      }
    }

    final onGifPaste = clipboardConfig?.onGifPaste;
    if (onGifPaste != null) {
      final gifBytes = await clipboardService.getGifFile();
      if (gifBytes != null) {
        final gifUrl = await onGifPaste(gifBytes);
        if (gifUrl != null) {
          replaceText(
            plainTextEditingValue.selection.end,
            0,
            BlockEmbed.image(gifUrl),
            null,
          );
          updateEditor?.call();
          return true;
        }
      }
    }

    // Only process plain text if no image/gif was pasted.
    // Snapshot the input before using `await`.
    // See https://github.com/flutter/flutter/issues/11427
    final plainText = (await Clipboard.getData(Clipboard.kTextPlain))?.text;

    if (plainText != null) {
      final plainTextToPaste = await getTextToPaste(plainText);
      if (pastePlainTextOrDelta(plainTextToPaste,
          pastePlainText: _pastePlainText, pasteDelta: _pasteDelta)) {
        updateEditor?.call();
        return true;
      }
    }

    final onUnprocessedPaste = clipboardConfig?.onUnprocessedPaste;
    if (onUnprocessedPaste != null) {
      if (await onUnprocessedPaste()) {
        updateEditor?.call();
        return true;
      }
    }

    return false;
  }

I suggest providing the option to override the canPaste state instead, so they don't need to depend on ClipboardMonitor.

I will then provide such option.
As an extra step, if we are keeping ClipboardMonitor, I suggest we optimize it at least to reduce the amount of checks. Can I make the timer to 5 seconds and prevent overlapping checks? (So it won't start checking if it didn't finish the previous one)

This reverts commit dac1cf6.

This reverts commit dfeb9c2.

Maksim-Nikolaev · 2025-01-05T21:02:23Z

Please check the most recent commit.
I added bool? enableClipboardPaste in lib\src\toolbar\config\simple_toolbar_config.dart for QuillSimpleToolbarConfig

This is passed directly to ClipboardButton with .paste action

As for the ClipboardButton itself and monitor:

enableClipboardPaste flag can be null or bool:

  /// If null, uses in-built [ClipboardMonitor]
  /// If true, paste button is enabled (if true & not readonly)
  /// If false, paste button is disabled
  final bool? enableClipboardPaste;

Updated ClipboardMonitor periodic timer from 1 second to 5 seconds
Update ClipboardMonitor to not overlap with previous task of itself (in case of big clipboard content)
Changed getter for "currentStateValue" to prioritize the enableClipboardPaste flag:

        return !controller.readOnly &&
            (kIsWeb || (widget.enableClipboardPaste ?? _monitor.canPaste));

EchoEllet · 2025-01-05T21:23:14Z

Updated ClipboardMonitor periodic timer from 1 second to 5 seconds

This is still a breaking change.
Could you minimize the changes to only override the can paste state (whether the button is enabled)?

I added bool? enableClipboardPaste in lib\src\toolbar\config\simple_toolbar_config.dart for QuillSimpleToolbarConfig

This option should be only in the button config, not the toolbar config.

Maksim-Nikolaev · 2025-01-05T21:27:01Z

This is still a breaking change.

Reverted to 1 second

Maksim-Nikolaev · 2025-01-05T21:37:32Z

This option should be only in the button config, not the toolbar config.

I agree that it would be the best way to do it. Unfortunately I couldn't find the proper place to add a new value, could you elaborate here?

  final QuillToolbarToggleStyleButtonOptions clipboardPaste;

import 'package:flutter/foundation.dart' show immutable;
import 'package:meta/meta.dart';

import '../base_button_options.dart';

class QuillToolbarToggleStyleButtonExtraOptions
    extends QuillToolbarBaseButtonExtraOptions
    implements QuillToolbarBaseButtonExtraOptionsIsToggled {
  const QuillToolbarToggleStyleButtonExtraOptions({
    required super.controller,
    required super.context,
    required super.onPressed,
    required this.isToggled,
  });

  @override
  final bool isToggled;
}

@immutable
class QuillToolbarToggleStyleButtonOptions
    extends QuillToolbarBaseButtonOptions<QuillToolbarToggleStyleButtonOptions,
        QuillToolbarToggleStyleButtonExtraOptions> {
  const QuillToolbarToggleStyleButtonOptions({
    super.iconData,
    super.iconSize,
    super.iconButtonFactor,
    super.tooltip,
    super.afterButtonPressed,
    super.iconTheme,
    super.childBuilder,
  });
}

Maksim-Nikolaev · 2025-01-05T21:52:58Z

Could you minimize the changes to only override the can paste state (whether the button is enabled)?

I would love to, but since it's coded in the way that we have little control over ClipboardMonitor it's difficult to update it's state or to stop the timer when the button already exists.

To stop the timer we need to trigger "didUpdateWidget" and add/remove extra listeners but it only happens when controller changes.
I kept the original handler to imitate the same behavior, as well as added some custom code to add/remove listener if "enableClipboardPaste" value changed from parent.

  @override
  void didUpdateWidget(QuillToolbarClipboardButton oldWidget) {
    super.didUpdateWidget(oldWidget);

    // Default didUpdateWidget handler, otherwise simple flag change didn't stop the monitor
    if (oldWidget.controller != controller) {
      oldWidget.controller.removeListener(didChangeEditingValue);
      removeExtraListener(oldWidget);
      controller.addListener(didChangeEditingValue);
      addExtraListener();
      currentValue = currentStateValue;
    }
    // If controller didn't change, but something else did (enableClipboardPaste)
    else if (widget.clipboardAction == ClipboardAction.paste) {
      // If enableClipboardPaste is not null, disable monitors
      if (widget.enableClipboardPaste != null) {
        _monitor.monitorClipboard(false, _listenClipboardStatus);
        currentValue = currentStateValue;
      } else {
        // Otherwise remove old listener and add a new one
        _monitor.monitorClipboard(true, _listenClipboardStatus);
        currentValue = currentStateValue;
      }
    }
  }

Maksim-Nikolaev · 2025-01-05T22:16:08Z

I think adding "isEnabled" to QuillToolbarBaseButtonOptions would be the best option, but this would require adding the functionality of this option to all the buttons. I can do it if you give me a green light.
Otherwise I'm open for recommendations how to handle it for just 1 type of button

Maksim-Nikolaev · 2025-01-05T23:13:56Z

Moved enableClipboardPaste to QuillToolbarBaseButtonOptions

On the video you can see that it works for all 3 cases (null, true, false) and the timer will continue as normal if it is set back to "null"
null = clipboardmonitor enabled
false = clipboard paste disabled, monitor disabled
true = clipboard paste enabled, monitor enabled

flutter.quill.clipboard.monitor.mp4

singerdmx · 2025-01-09T03:37:54Z

@EchoEllet can we merge this?

EchoEllet · 2025-01-09T04:49:52Z

Updated ClipboardMonitor periodic timer from 1 second to 5 seconds

This is still a breaking change.
Could you minimize the changes to only override the can paste state (whether the button is enabled)?

I added bool? enableClipboardPaste in lib\src\toolbar\config\simple_toolbar_config.dart for QuillSimpleToolbarConfig

This option should be only in the button config, not the toolbar config.

can we merge this?

I will work on it soon. However, it's not a high-priorty issue given that I have disabled the buttons by default (revert to the default behavior).

Maksim-Nikolaev added 3 commits January 5, 2025 19:45

Removed ClipboardMonitor from ClipboardAction.paste type button

dac1cf6

Update CHANGELOG.md

fb3f2d8

Removed unused imports & added const keyword

dfeb9c2

Maksim-Nikolaev added 3 commits January 5, 2025 20:20

Revert "Removed ClipboardMonitor from ClipboardAction.paste type button"

6ba6119

This reverts commit dac1cf6.

Revert "Removed unused imports & added const keyword"

dc4019a

This reverts commit dfeb9c2.

added enableClipboardPaste to QuillSimpleToolbarConfig

f2f3d86

Reverted 5 seconds to 1 second periodic timer

7727f08

Maksim-Nikolaev changed the title ~~Removed ClipboardMonitor from ClipboardAction.paste type button~~ Added an option for ClipboardPaste button to disable ClipboardMonitor Jan 5, 2025

Maksim-Nikolaev added 2 commits January 5, 2025 23:47

moved enableClipboardPaste to button config

eb0ddeb

Fix monitor enabling & disabling

ac00d45

Maksim-Nikolaev added 2 commits January 6, 2025 00:16

dartformat

956b2d2

remove enableClipboardPaste from toolbar config

b6e343d

singerdmx approved these changes Jan 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added an option for ClipboardPaste button to disable ClipboardMonitor #2427

Added an option for ClipboardPaste button to disable ClipboardMonitor #2427

Maksim-Nikolaev commented Jan 5, 2025 •

edited

Loading

EchoEllet commented Jan 5, 2025 •

edited

Loading

Maksim-Nikolaev commented Jan 5, 2025

Maksim-Nikolaev commented Jan 5, 2025

EchoEllet commented Jan 5, 2025

Maksim-Nikolaev commented Jan 5, 2025

Maksim-Nikolaev commented Jan 5, 2025

Maksim-Nikolaev commented Jan 5, 2025 •

edited

Loading

Maksim-Nikolaev commented Jan 5, 2025

Maksim-Nikolaev commented Jan 5, 2025 •

edited

Loading

singerdmx commented Jan 9, 2025

EchoEllet commented Jan 9, 2025

Added an option for ClipboardPaste button to disable ClipboardMonitor #2427

Are you sure you want to change the base?

Added an option for ClipboardPaste button to disable ClipboardMonitor #2427

Conversation

Maksim-Nikolaev commented Jan 5, 2025 • edited Loading

Description

Related Issues

Type of Change

EchoEllet commented Jan 5, 2025 • edited Loading

Maksim-Nikolaev commented Jan 5, 2025

Maksim-Nikolaev commented Jan 5, 2025

EchoEllet commented Jan 5, 2025

Maksim-Nikolaev commented Jan 5, 2025

Maksim-Nikolaev commented Jan 5, 2025

Maksim-Nikolaev commented Jan 5, 2025 • edited Loading

Maksim-Nikolaev commented Jan 5, 2025

Maksim-Nikolaev commented Jan 5, 2025 • edited Loading

singerdmx commented Jan 9, 2025

EchoEllet commented Jan 9, 2025

Maksim-Nikolaev commented Jan 5, 2025 •

edited

Loading

EchoEllet commented Jan 5, 2025 •

edited

Loading

Maksim-Nikolaev commented Jan 5, 2025 •

edited

Loading

Maksim-Nikolaev commented Jan 5, 2025 •

edited

Loading