Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added exception handling to color parsing #12

Merged
merged 1 commit into from
Nov 3, 2024

Conversation

mtallenca
Copy link
Contributor

Added exception handling when parsing a color value. Saw the following css color value on a website...

color: rgb(0 0 0/var(--tw-text-opacity));

Looks like something from Tailwind CSS. In any case, released code throws an exception and clipboard paste is halted.

@CatHood0 CatHood0 merged commit 88410f6 into CatHood0:master Nov 3, 2024
@mtallenca mtallenca deleted the catch_color branch November 4, 2024 01:58
@EchoEllet
Copy link
Contributor

EchoEllet commented Nov 4, 2024

color: rgb(0 0 0/var(--tw-text-opacity));

It might be a good idea to comment on why the code returns null in case of any exception and when it throws an exception. Also, some tests cover this case so the fix won't removed accidentally.

@CatHood0
Copy link
Owner

CatHood0 commented Nov 4, 2024

It might be a good idea to comment on why the code returns null in case of any exception and when it throws an exception. Also, some tests cover this case so the fix won't removed accidentally.

We could do better by creating exceptions controlled by the developer himself. That is, adding functions so that in any case in which an exception is thrown, the developer is the one who decides what to do with the object that is going to be given to him (whether he will do a rethrow, or show his own custom exception, or just show a message)

@mtallenca
Copy link
Contributor Author

In my case, someone was trying to paste a blob of html into flutter_quill editor and one color style was causing the exception so nothing was pasted. Not sure a calling application would have any idea how to handle the exception, at a minimum the text should paste, which is what happens with the current try / catch.

@EchoEllet
Copy link
Contributor

EchoEllet commented Nov 4, 2024

In my case, someone was trying to paste a blob of html into flutter_quill editor and one color style was causing the exception so nothing was pasted

Maybe we could paste as plain text when in case of a failure. I'm thinking about adding paste as a plain text option in the context menu or disabling the rich text paste by default completely in v11 as default.

In either case, in v11, it's possible to override the default without rewriting it completely.

@EchoEllet
Copy link
Contributor

EchoEllet commented Nov 4, 2024

In my case, someone was trying to paste a blob of html into flutter_quill editor and one color style was causing the exception so nothing was pasted

Maybe we could paste as plain text when in case of a failure. I'm thinking about adding paste as a plain text feature or disabling the rich text paste by default completely.

I welcome suggestions.

In either case, in v11, it's possible to override the default without rewriting it completely.

@mtallenca
Copy link
Contributor Author

mtallenca commented Nov 4, 2024

I do like the the html paste but need control over what attributes are allowed as our editor implementation doesn't allow all of the editor features. Having a plain text feature or a callback to edit pasted delta would be better than what I'm doing now.

`
onReplaceText: (index, len, data) {
var changed = false;
final operations = [];

      if (data is Delta) {
        for (final op in data.operations) {
          final attributes = <String, dynamic>{};
          for (final key in op.attributes?.keys ?? <String>[]) {
            if (!['color', 'font', 'background', 'link', 'size', 'line-height', 'header']
                .contains(key)) {
              attributes[key] = op.attributes![key];
            } else {
              changed = true;
            }
          }
          operations.add(Operation.insert(op.data, attributes.isEmpty ? null : attributes));
        }

        if (changed) {
          WidgetsBinding.instance.addPostFrameCallback((_) {
            final newDelta = Delta.fromOperations(operations);
            _controller!.replaceText(index, len, newDelta, _controller!.selection);
          });

          return false;
        }
      }

      return true;
    },`

@EchoEllet
Copy link
Contributor

Can file an issue with the details on Flutter Quill repo? So we don't lose track of it, with your suggestion, and what do you think would be better with an example.

I plan on looking at this issue soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants