flarum / framework

Simple forum software for building great communities.
http://flarum.org/
6.28k stars 826 forks source link

Issue when writing a post: ctrl + z issue #1782

Closed PeopleInside closed 3 years ago

PeopleInside commented 5 years ago

Bug Report

Current Behavior Cannot underdone more than one time message errors with CTRL + Z

Sometime as staff but also as member ( admin and user side ) you need to write a long post. If you do errors and use ctrl + z for try to go back you can only resume one time...

For example if now i write 5 sentences here (1,2,3,4,5) than need to undone the 5 and 4 and 3 i cannot do with ctrl + z just sentence 5 will be removed.

first sentence
second sentence
third sentence
fourth sentence
fifth sentence

Now i press ctrl + z i should be able to undone different changes... but is allowed just one.

This can be a big issue as you can done mistakes and you can be unable to solve. I believe this should be reported on GitHub.

Current Behavior

Issue when writing a post: ctrl + z issue

Steps to Reproduce Go to the editor and write a message with 5 different sentence one for row. try to undo the last two or three sentence with ctrl + z , you are able to undone just the last changes done. If you are unable to do many undo you can loose work on a long text where you do errors and need undo more than one step.

Expected Behavior Press ctrl + z and undo the last action, press again and undo another previous action, etc.

Environment

Flarum core 0.1.0-beta.8.1
PHP version: 7.3.X
Loaded extensions: Core, date, libxml, pcre, dom, fileinfo, filter, hash, json, SPL, bz2, posix, Reflection, session, SimpleXML, standard, xml, xmlreader, xmlwriter, bcmath, openssl, calendar, ctype, curl, dba, mbstring, ftp, gd, gettext, gmp, iconv, imap, intl, ldap, exif, zlib, pcntl, PDO, pdo_sqlite, shmop, soap, sockets, sqlite3, sysvsem, sysvshm, tidy, tokenizer, wddx, xmlrpc, xsl, zip, Phar, mysqlnd, mysqli, pdo_mysql, ionCube Loader, Zend OPcache
+------------------------------+-----------------+--------+
| Flarum Extensions            |                 |        |
+------------------------------+-----------------+--------+
| ID                           | Version         | Commit |
+------------------------------+-----------------+--------+
| flarum-approval              | v0.1.0-beta.8   |        |
| flarum-bbcode                | v0.1.0-beta.8   |        |
| flarum-emoji                 | v0.1.0-beta.8   |        |
| flarum-lang-english          | v0.1.0-beta.8   |        |
| flarum-flags                 | v0.1.0-beta.8.1 |        |
| flarum-likes                 | v0.1.0-beta.8.1 |        |
| flarum-lock                  | v0.1.0-beta.8   |        |
| flarum-markdown              | v0.1.0-beta.8   |        |
| flarum-mentions              | v0.1.0-beta.8.1 |        |
| flarum-statistics            | v0.1.0-beta.8   |        |
| flarum-sticky                | v0.1.0-beta.8   |        |
| flarum-subscriptions         | v0.1.0-beta.8   |        |
| flarum-suspend               | v0.1.0-beta.8   |        |
| flarum-tags                  | v0.1.0-beta.8.2 |        |
| flarum-pusher                | v0.1.0-beta.8.1 |        |
| nearata-lang-italian         | v0.1.0-beta.8.2 |        |
| flagrow-upload               | 0.7.1           |        |
| fof-user-bio                 | 0.1.2           |        |
| fof-default-user-preferences | 0.1.0           |        |
| fof-links                    | 0.2.0           |        |
| fof-recaptcha                | 0.1.0           |        |
| flarum-akismet               | v0.1.0-beta.8   |        |
| flagrow-terms                | 0.3.1           |        |
| flarum-auth-facebook         | v0.1.0-beta.8   |        |
| flarum-auth-github           | v0.1.0-beta.8   |        |
| flarum-auth-twitter          | v0.1.0-beta.8   |        |
+------------------------------+-----------------+--------+
Base URL: https://community.marcoborla.it

https://discuss.flarum.org/d/20044-issue-when-writing-a-post-ctrl-z-issue

Can be reproduced also on the official Flarum community.

luceos commented 5 years ago

I'm not entirely sure but isn't this the responsibility of the browser or os?

PeopleInside commented 5 years ago

I'm not entirely sure but isn't this the responsibility of the browser or os?

Is not the browser i am trying on other forums (not Flarum) and is working as expected... so i can undone more than the last edit done. The issue is present only on Flarum.

clarkwinkelmann commented 5 years ago

Maybe be related to the two-way bindings of Mithril that could interfere with the browser cancel feature ?

Can someone test how it behaves in other browsers ? Do we see the same issue ?

PeopleInside commented 5 years ago

@clarkwinkelmann tested now and it's the same with Chrome

jordanjay29 commented 5 years ago

I can confirm this behavior as well. It appears that a linebreak cuts the ability to ctrl-z undo before the linebreak. Anything on the same line/paragraph has full ctrl-z/shift-ctrl-z capabilities afaict.

franzliedke commented 5 years ago

Could this be caused by the markdown toolbar?

PeopleInside commented 5 years ago

Hi @franzliedke , maybe from some test seems can be the markdown tried with and without and seems ctrl + z is working better without.

askvortsov1 commented 4 years ago

If https://github.com/jahudka/mdarea/issues/4 is implemented, that should resolve this issue.

jahudka commented 4 years ago

Hi, I'm the developer of mdarea, just thought I'd chime in to let you guys know I'm aware of the issue - I won't go into too much detail here, but it appears it could be done - read the comments of the issue @askvortsov1 mentioned for more info, esp. this one - but it might increase the package size considerably because it's not trivial to implement. If that's not an issue in Flarum's context then I'll be happy to give it a go when I find the time.

askvortsov1 commented 3 years ago

If we end up offering a full WYSIWYG editor via-extension in https://github.com/flarum/core/issues/2566, I think we can consider this effectively fixed.

jahudka commented 3 years ago

Hi, I've looked into this again and I'm afraid it's even harder to fix than I thought.. I've tried going the route we discussed in the mdarea issue, but it relies on execCommand which is impossible to get to behave consistently across browsers - for example I have a version of the implementation now which works flawlessly in Opera, but redo breaks in Safari and everything breaks in Firefox.. I'm sorry, but I'd rather wait for native undo and then implement this in one line than wrestle with execCommand for an unholy amount of time only to find it can't be done anyway (or that it would increase the bundle size 10x).

jahudka commented 3 years ago

Actually, you know what? I've solved it. πŸ˜‚πŸ˜‚πŸ˜‚ I couldn't help myself πŸ˜‚ Check out mdarea version 2.0. It even has built-in typings now. I tested it on macOS in Safari, Opera, Firefox and Chrome and it seems to work. Gzipped bundle size went up from ~1.7KB to ~2.5KB, but I think that's an acceptable tradeoff to get full undo.

https://jahudka.github.io/mdarea/

askvortsov1 commented 3 years ago

The new Typescript version looks great! The class-based refactoring in particular means it should be very extensible, which works great with our extension api! We're hoping to redesign our editor system a bit to allow customization of the editor via drivers, and with that, I'm hoping to bring Flarum's use of mdarea from markdown into core for the basic editor, since the list editor is quite useful regardless of markdown (and we might want to reintroduce some inlines like quotes and backticks).

That being said, the undo seems to act up a bit as transactions stack up: if I type some lines, then delete something, it doesn't restore correctly, or tries to copy-paste the entire contents of the text editor.

jahudka commented 3 years ago

Hey, sorry for the delay, thanks for the feedback, I've tested it pretty extensively before publishing version 2.0, I don't know how I could've missed this.. anyway, could you please try the demo now? I've changed some more stuff & tested that the demo works in all the major browsers.

As for the extensibility - mdarea isn't very extensible right now, I'm afraid. The class is mainly the API visible from the outside; internally most work is done by closures which you can't easily replace from the outside. The reason for this is that I've been trying to bring down the package size as much as possible - so I've been avoiding class methods and properties (because they'd need this. and their name wouldn't be mangled to a single character). So for now you can play around with the options, but that's pretty much it.. If you need to be able to override / extend something specific, let me know, I'll look into it. I'm also thinking of extracting some of the behaviour as submodules, so that if you know you'll only be using e.g. the list functionality you can include just that and cut down on the bundle size even further. We'll see..

askvortsov1 commented 3 years ago

Wow, it's working VERY well, this is awesome! Thank you so much for all your work on this. Thinking over it again, I don't think there's anything in particular we'd need to extend; I want to include it in core because the markdown it supports semantically makes sense without formatting, so it's overall a much nicer typing experience. And add-ons to markdown that wouldn't be supported by mdarea by default generally don't really retain that semantic benefit.

jahudka commented 3 years ago

Hey, I've made a couple more updates - mostly to allow mdarea to have plugins, but I fixed a couple of bugs as well. The plugin API is very minimal, essentially it just calls a plugin method on every keystroke, allowing the plugin to take action before defaulting to the built-in behaviour - but the integration allows plugins to leverage the internal undo stack.

The reason I did that is this: https://jahudka.github.io/mdarea/suggest.html

It's what I call a minimal UI.

It's something I need anyway and I remember reading somewhere that Flarum needs mentions.. this should work for those, among other things.

askvortsov1 commented 3 years ago

We'll need to wait till https://github.com/flarum/core/pull/2594 is merged to update. In core we instantiate the textarea with the value by passing it in as a mithril attr. This applies the value on every redraw, which throws off mdarea. With the PR I linked, the textarea will be added to the DOM dynamically, which eliminates this issue. Not a bug with mdarea, just noting why we can't update this yet.

jahudka commented 3 years ago

Hi, just one more thing: the "autosuggest" feature linked in the demo above has now been released as a separate package called mdarea-suggest. It should be an okay super-minimalistic interface on top of which you can implement @mentions, #hashtags and similar stuff. The "production" demo of the package is available here.

PeopleInside commented 3 years ago

Hi, is normal that if Enable mdarea? is disabled in the Markdown extension for example for the bug reported here https://github.com/flarum/core/issues/2746

using Ctrl + Z never work? Means all text you digit is erased and quoted text cannot be undone so if you have many rows it's an issue undone this.

Also from my test seems Ctrl + Y is not supported. Is not possible to resume text.

Maybe this issue should be re-opened or you need fix some other things regarding Markdown and regarding Ctrl + Z and Ctrl + Y

If you digit in a post: word1 word2 word3 word4 word5 word6 speared by the space than press Ctrl + z the full row is erased.

Tested; https://beta.flarum.site

PS: Need clear cache every time the " Enable mdarea? " is changed and saved or is not applied to the community. I cannot reopen this issue as I have no permission for that.

PeopleInside commented 3 years ago

1

With the settings above, results:

2

With the option enabled as the below screenshot:

3

Results:

4

I think this or a new issue should be reopen or open. Let me know as I want monitor those issues.

askvortsov1 commented 3 years ago

using Ctrl + Z never work? Means all text you digit is erased and quoted text cannot be undone so if you have many rows it's an issue undone this.

Certain operations (like quoting) do not support ctrl+z. This is possible on more advanced editors (eg. the rich text extension), but not on the simple textarea one.

PeopleInside commented 3 years ago

Certain operations (like quoting) do not support ctrl+z. This is possible on more advanced editors (eg. the rich text extension), but not on the simple textarea one.

So I need to try to install https://discuss.flarum.org/d/26455-wysiwyg-rich-text-editor to resolve the impossibility of undo quoting text?

How about:

Is also normal that deactivating " Enable mdarea? " give maybe back to the situation that generated this issue? Try to watch the gift I attached above.

thanks for your help.

askvortsov1 commented 3 years ago

Ctrl + Y that doesn't work

I am unable to replicate this, I'm not quite seeing it in your video. If you type anything (spaces, characters, delete, newline, etc), all Ctrl + Y history will be lost. That's just how text editors work. The same applies in this github editor.

More words in a sentence undone will erase the full sentence not the single word without the possibility to restore with Ctrl + Y

To an extent, that's just how the browser's undo/redo system works. Isn't this the same as in any textarea element?

PeopleInside commented 3 years ago

With the rich text editor is better, I will keep the plugin... maybe did not solve all issues but you are right.. also here in GitHub the editor is not perfect on undo, etc. Thanks