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

Enable TextTransformation in all builds? #1843

Closed
Reinmar opened this issue Jun 28, 2019 · 10 comments
Closed

Enable TextTransformation in all builds? #1843

Reinmar opened this issue Jun 28, 2019 · 10 comments
Labels
resolution:expired This issue was closed due to lack of feedback. squad:core Issue to be handled by the Core team. status:discussion status:stale type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@Reinmar
Copy link
Member

Reinmar commented Jun 28, 2019

Once we have a confirmation that this feature works reliably with the variety of our plugins and integrations, it'd most likely make sense to enable it by default.

The downsides:

  • code size (the definition of transformations take some space),
  • will change how typing works and may force some people to change their integrations (remove this plugin),
  • most people will not ever notice we enabled it.

Arguments for:

  • leaves a really nice feeling when you realise that the editor improves the typography for you.

Discuss :D

@Reinmar Reinmar added status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option). labels Jun 28, 2019
@Reinmar Reinmar added this to the iteration 26 milestone Jun 28, 2019
@wwalc
Copy link
Member

wwalc commented Jun 28, 2019

What's the size difference in KB (gzipped) in the classic build?

@jodator
Copy link
Contributor

jodator commented Jul 1, 2019

Side note on such questions: creating an online builder - that was discussed somewhere already - could help us solving such issues. I think that having such tools would allow us to see what people are adding/removing to/from builds.

@Reinmar
Copy link
Member Author

Reinmar commented Aug 12, 2019

I'd sync this change with #1844, so postponing.

@Reinmar Reinmar modified the milestones: iteration 26, iteration 27 Aug 12, 2019
@Reinmar Reinmar self-assigned this Aug 27, 2019
@Reinmar
Copy link
Member Author

Reinmar commented Sep 19, 2019

Results without and with this plugin:

-rw-r--r--   1 p  staff   579727 Sep 19 21:41 ckeditor.js
-rw-r--r--   1 p  staff   148006 Sep 19 21:41 ckeditor.js.gz

-rw-r--r--   1 p  staff   583153 Sep 19 21:43 ckeditor.js
-rw-r--r--   1 p  staff   149282 Sep 19 21:43 ckeditor.js.gz

@Reinmar
Copy link
Member Author

Reinmar commented Sep 19, 2019

But I checked its popularity and I'd say it's similar to that of mentions, word count, remove formatting, etc. So why not add the other ones too?

From a different perspective, the text transformation feature adds a "nice touch" and should be well received by people who care about typography. But for them to notice it, it'd have to be enabled by default. And that's quite a significant change. I'd say it's a breaking change because this is a quite intrusive plugin.

Also, I think the plugin is far from ideal. The transformations should be localized – different languages have different typographical solutions. This plugin isn't awere of that. It also doesn't handle well the situation when the user want to revert the transformation (the plugin is stubborn and will fix your text infinitely). That was reported to us, in fact.

So, there are two options:

  1. Enable it in the documentation only. See if there's any reaction.
  2. Do nothing now and wait for a bigger release of CKEditor 5 to add it.

@Reinmar Reinmar modified the milestones: iteration 27, nice-to-have Sep 19, 2019
@jodator
Copy link
Contributor

jodator commented Sep 20, 2019

doesn't handle well the situation when the user want to revert the transformation (the plugin is stubborn and will fix your text infinitely).

@Reinmar: Could you explain this more? How this happens? You should be able to undo a transformation after it happens. After that should be able to type and the text should remain unfixed.

@Reinmar
Copy link
Member Author

Reinmar commented Oct 8, 2019

I think one of our customers was reporting this (when used with autocapitalization transformation, but applies to other ones too):

  1. Type .
  2. Type a. It becomes A.
  3. So, to revert that you press Backspace and a again.
  4. It gets fixed again.

@jodator
Copy link
Contributor

jodator commented Oct 9, 2019

Well, this case cannot be detected as "undoing" easily. typing a after a . is in both situations the same. We do not apply any heuristics to this behavior and I really don't see a need for such. We add "undo" for this. Also this case:

  1. Type *foo.
  2. Type * (it becomes bolded).
  3. Undo (it becomes normal text *foo*.
  4. You've changed your mind and wish to have bold again.
  5. Press Backspace to delete *.
  6. Press * again.
  7. The text is bolded.

Same steps - different expectations. I think that Undo is a way to go and Backspace shouldn't block the next transformation.

@Reinmar Reinmar added the squad:core Issue to be handled by the Core team. label Aug 18, 2020
@Reinmar Reinmar removed their assignment Aug 18, 2020
@pomek pomek removed this from the nice-to-have milestone Feb 21, 2022
@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added the resolution:expired This issue was closed due to lack of feedback. label Nov 4, 2023
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution:expired This issue was closed due to lack of feedback. squad:core Issue to be handled by the Core team. status:discussion status:stale type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

No branches or pull requests

5 participants