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

Classic Block - Insert Media Icon - New Upload jumps to top #10509

Closed
La-Geek opened this issue Oct 11, 2018 · 23 comments · Fixed by #12415
Closed

Classic Block - Insert Media Icon - New Upload jumps to top #10509

La-Geek opened this issue Oct 11, 2018 · 23 comments · Fixed by #12415
Assignees
Labels
[Block] Classic Affects the Classic Editor Block [Feature] Blocks Overall functionality of blocks [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@La-Geek
Copy link

La-Geek commented Oct 11, 2018

When you insert a media file, e. g. an image with the insert media icon
then hit return
and insert a second image the same way, the new image jumps to top, But it should be below the first image.

mediajump

Desktop (please complete the following information):

  • OS: Windows 10 (recent build) 64bit
  • Browser Chrome (recent build)
  • Version Gutenberg pre-release download package
@Soean Soean added [Type] Bug An existing feature does not function as intended [Feature] Blocks Overall functionality of blocks labels Oct 11, 2018
@La-Geek
Copy link
Author

La-Geek commented Oct 21, 2018

Bug is also in Gutenberg 4.1

@designsimply
Copy link
Member

Thank you for the report! I'm closing this issue now because I have just tested with WordPress 4.9.8 and Gutenberg 4.2.0-rc.1 and did not see the problem happen there. (22s) I used Firefox 63.0 on macOS 10.13.6 for testing. If you are still seeing the problem either after testing with 4.2.0-rc.1 or using the 4.2 version after it is released, please let me know and we can re-open!

@La-Geek
Copy link
Author

La-Geek commented Nov 2, 2018

The problem persists with 4.9.8 and with this Gutenberg version: https://github.com/WordPress/gutenberg/releases/download/v4.2.0-rc.1/gutenberg.zip

insert an image, write a word, insert next image and this image will be on top and not below.

@La-Geek
Copy link
Author

La-Geek commented Nov 4, 2018

odd_behavior

Fresh installation of WP 4.9.8 and Gutenberg 4.2.0.-rc.1 and nothing else

@earnjam earnjam reopened this Nov 4, 2018
@earnjam
Copy link
Contributor

earnjam commented Nov 4, 2018

I am able to recreate this. It actually occurred for me in Firefox as well. Every time I inserted an image, it was inserted at the beginning of the block content, regardless of cursor location

classic-block-image-insert

Using:
WP: 5.0-beta2-43852
Gutenberg: 4.2.0-rc.1
Chrome: 70 OR Firefox 63
OS X

@La-Geek
Copy link
Author

La-Geek commented Nov 8, 2018

It would be awesome, if this could be set to your next milestone. This is a strange bug and make the block classic useless for inserting more than one image.

@azaozz
Copy link
Contributor

azaozz commented Nov 16, 2018

I think I found the problem. It's the (re)inserting of the content every time componentDidUpdate runs, here.

Trying few workarounds but because the above runs quite often, and because we save the content every time on editor blur, which also happens when an image is selected from the media modal, using the standard TInyMCE way of getBookmark(), moveToBookmark() often fails (the bookmark is overwritten on the second set of save/reinsert).

Looking at other possible workarounds as this is really annoying bug, especially when the content in the Classic Block is longer.

@youknowriad
Copy link
Contributor

@azaozz Disabling the "reiniserting" doesn't fix the issue for me. Any more insights on this?

The workaround for now is to insert an image block between two classic blocks if needed but would be good to fix anyway.

@La-Geek
Copy link
Author

La-Geek commented Nov 20, 2018

The workaround for now is to insert an image block between two classic blocks

I disagree, this is no solution. Non experienced users should have the chance to learn the block editor bit by bit. The block classic should work to 100 percent like the classic editor does now. And imo, the block editor will be the most used block on start.

And an image block is no inline image.

@youknowriad
Copy link
Contributor

Worth nothing that you can inline images into paragraph blocks as well.

@La-Geek
Copy link
Author

La-Geek commented Nov 20, 2018

Inline images with very poor functions. You can't align the image, you can't link the image or edit it.
And block paragraph does not replace a block classic.

When you align it, you will align the text too.
When you set an URL to the image, you'll get something like taht:

<p style="text-align:left">test<img class="wp-image-8" style="width: 150px;" src="http://link.de/gttest/wp-content/uploads/2018/11/IMG_0386a.png" alt=""><a href="https://google.de"></a></p>

@azaozz
Copy link
Contributor

azaozz commented Nov 20, 2018

@youknowriad we've been looking into this with @iseulde. Disabling the "reiniserting" doesn't fix it completely, still need to add the TinyMCE way of keeping the cursor position, i.e. do the getBookmark(), moveToBookmark() dance. Then it seems to work. However there are other "side effects" :(

We'll get to the bottom of this! :)

@youknowriad
Copy link
Contributor

Thanks for the updates, let's get this in as soon as we can.

@turtlepod
Copy link

@azaozz I create a custom classic editor component with custom media button. I use mceInsertContent not sure if this is the right way (to create custom callback for classic editor media button, but seem to work fine. thoughts?

editor.execCommand( 'mceInsertContent', false, renderToString( imageTag( mediaAttachment ) ) );

https://github.com/turtlepod/fx-classic-editor/blob/master/components/fx-classic-editor/index.js#L166

@designsimply
Copy link
Member

@La-Geek apologies for closing the issue so soon before. I did test and couldn't see the problem happening with 4.0-rc.1 at that time, but then I saw that earnjam re-opened. I re-tested again just now with WordPress 4.9.8 and Gutenberg 53e4c40bd using Firefox 63.0.1 on macOS 10.13.6 after working on another issue with media in the classic block and found that any time I try to insert an image into a classic block it is added to the top instead of where the cursor was placed. (16s)

Another workaround idea is to insert and image and then cut and paste it into the location where you want the image to appear (this is just a potential workaround to use while the bug is being worked on).

@La-Geek
Copy link
Author

La-Geek commented Nov 21, 2018

No problem @designsimply 👍

However such workarounds are no good idea. Unexperienced users will work with the block classic (imho) and learn the other blocks bit by bit. I dislike to teach my client to copy and paste images (and how this should work with images from media manager)?

@azaozz
Copy link
Contributor

azaozz commented Nov 28, 2018

Please test #12393 if possible, with images and with other (custom) TinyMCE plugins that insert content. Will probably fix or improve other issues when inserting or updating content in the editor.

@turtlepod seems this will work, but why replace the "standard" way of inserting images? The biggest problem here is that the Classic Block looses the caret position on blur, and on top of that the whole editor content is getting refreshed. This is pretty similar to editing text in a textarea: imagine you do select-all, copy, delete all, paste. Of course the caret position is gone after that :)

@La-Geek
Copy link
Author

La-Geek commented Nov 28, 2018

Unfortunately I am unable to test it. I don't have a locale installation, grunt etc. I test with a .htaccess protected installation (online) and Gutenberg 4.5.1.
I copied the folder classic including all files except the complete folder "test" into the folder "src" path /wp-content/plugins/gutenberg/packages/block-library/src

The image still is jumping.

I did a second "test" and copied the folder classic into the folder "build", path wp-content/plugins/gutenberg/build

The images still is jumping.

No idea, if my test workaround made any sense :)

@azaozz
Copy link
Contributor

azaozz commented Nov 29, 2018

@La-Geek ah, I'm sorry, I should have explained better.

To be able to test the PR you need to "build" Gutenberg. For that you would need the dev. environment, git, node.js, npm, etc. IMHO not worth it setting it up if you're not coding.

On the other hand, if you would like to test PRs and get more involved in the development (and know your way with computers), setting a dev. environment shouldn't be hard. Here is a guide: https://github.com/WordPress/gutenberg/blob/master/CONTRIBUTING.md.

@azaozz
Copy link
Contributor

azaozz commented Nov 29, 2018

Closed #12393 in favor of #12415. It's simpler and does the same thing, stores the selection on blur, then restores it on focus.

@gschoppe
Copy link

gschoppe commented Dec 5, 2018

@mtias This is a breaking issue for the classic block, which is meant to be the entry point for users learning to work in the new system. The functionality of this block is crucial for a smooth launch. How can this possibly be justified as a 5.0.1 fix? By that time, thousands of users will have had their first experience of the new editor be that it can't even add a simple image properly.

Launching without a fix for this issue in place is just asking for community backlash.

@turtlepod
Copy link

I agree with @gschoppe
Related issue #7932

@sawtoothid
Copy link

@gschoppe Hit the nail square on the head.

I am one of the thousands (dare I speculate hundreds of thousands?) of users that experienced this. Allowing a general release with this kind of bug is a travesty.

I think this is going to generate a HUGE backlash, as basically it is now impossible to edit existing posts/pages with images using Gutenberg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Classic Affects the Classic Editor Block [Feature] Blocks Overall functionality of blocks [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.