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.

Sign up for GitHub

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

Jump to bottom

TRY: Text Block: Sometimes the cursor gets moved behind the first character #2517

Closed
wants to merge 2 commits into from
Closed

TRY: Text Block: Sometimes the cursor gets moved behind the first character #2517

wants to merge 2 commits into from

Conversation

tg-ephox
Copy link
Contributor

fixes #1607

Have investigated this issue a lot and was un-able to replicate in plain JS or React outside of Gutenberg. Safari behaves differently than FireFox and Chrome in that it leaves the cursor in the block when the link or button is clicked, and when typing is started again in the block after focus has moved, the selection point is lost maybe?

This fix is a little heavy handed but makes the behaviour the same as FireFox and Chrome, in that when a link/button is clicked, the cursor does not remain in the block.

@codecov
Copy link

codecov bot commented Aug 24, 2017 •
edited
Loading

Codecov Report

Merging #2517 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2517      +/-   ##
==========================================
- Coverage   31.42%   31.42%   -0.01%     
==========================================
  Files         177      177              
  Lines        5409     5410       +1     
  Branches      946      946              
==========================================
  Hits         1700     1700              
- Misses       3136     3137       +1     
  Partials      573      573
Impacted Files Coverage Δ
blocks/editable/index.js 10.48% <0%> (-0.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75d6f46...575e41c. Read the comment docs.

@tg-ephox tg-ephox requested a review from youknowriad August 24, 2017 01:39
@youknowriad
Copy link
Contributor

Not an easy issue indeed. I think the fix here, breaks the formatting toolbar. Sometimes clicking formatting buttons changes the position of the caret.

@karmatosed
Copy link
Member

@tg-ephox could you iterate based on feedback for this as would love to get this into 1.1.

@karmatosed karmatosed added the [Priority] High Used to indicate top priority items that need quick attention label Aug 30, 2017
@tg-ephox
Copy link
Contributor Author

@youknowriad @karmatosed have made some progress on this. Removed all of Gutenberg/React from createEditorInstance (main Gutenberg entry point) so that its just using some plain javascript to render a TinyMCE instance and a button and the issue still occurs! My suspicion is that a plugin or another JS library is causing this, as I can't replicate it outside of Wordpress. Will start removing JS libraries next!

@tg-ephox
Copy link
Contributor Author

tg-ephox commented Sep 1, 2017

@youknowriad @karmatosed this could be a TinyMCE/Safari bug... I do however have a fix but its outside the realm of Gutenberg! If the tabindex="0" is removed from the div with id wpbody-content this issue no longer occurs.

screen shot 2017-09-01 at 3 23 37 pm

Issue can be replicated with this page. Click on the content to focus the editor, then click the button and type again.

<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>title</title>
    <meta name="viewport" content="width=device-width,initial-scale=1.0">
    <script src='https://cloud.tinymce.com/stable/tinymce.min.js'></script>
  </head>
  <body>
        <div class="gutenberg__editor" tabindex="0">
            <p id="editor" contenteditable="true">Here is some content</p>
            <button id="button">Click Me</button>
        </div>
        <script type='text/javascript'>
            document.getElementById('button').onclick = (event) => event.preventDefault();

            const tinySettings = {
                theme: false,
                inline: true,
                toolbar: false,
                browser_spellcheck: true,
                entity_encoding: 'raw',
                convert_urls: false,
                plugins: [],
                formats: {
                    strikethrough: { inline: 'del' },
                },
                target: document.getElementById('editor'),
                setup: ( editor ) => {
                },
            };

            tinymce.init(tinySettings);
        </script>
        </body>
</html>

@youknowriad
Copy link
Contributor

Does this mean the issue can be replicated in the current WP editor?

@tg-ephox
Copy link
Contributor Author

tg-ephox commented Sep 5, 2017

@youknowriad this doesn't appear to happen in current WP editor (typing then clicking the Preview button) but im not sure that the Preview button handler calls event.preventDefault. I noticed also that the TinyMCE instance in the current editor is in an iframe, so this would have differences with selection and focus also... Here is a TinyMCE fiddle (Safari only) that replicates the issue also.

@anna-harrison
Copy link

Hey @youknowriad were you able to remove tabIndex from "wpbody-content" (see highlighted part above)? Thx

@youknowriad
Copy link
Contributor

youknowriad commented Sep 16, 2017 •
edited
Loading

@annaephox no, we're not wpbody-content is hard coded in the WordPress admin-header file. I think we should investigate the root problem in TinyMCE, but it's ok for me to add a temporary commented javascript hack to remove this attribute in createEditorInstance call. Not sure if it have an impact on the UI (styling) though, we'll have to check.

@anna-harrison
Copy link

@youknowriad : this issue is fixed in TinyMCE 4.7
Can we close this off?

@anna-harrison
Copy link

@tg-ephox : reminder to test this again when Gutenberg updates to TinyMCE 4.7

@youknowriad
Copy link
Contributor

@annaephox Great news, I tested on #2787 and it works like a charm, no bug any more. 🎉 Thanks for the fix

@youknowriad youknowriad closed this Oct 6, 2017
@youknowriad youknowriad deleted the try/1607-cursor-moves-behind-first-character branch October 6, 2017 09:17
@aduth aduth mentioned this pull request Nov 9, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youknowriad youknowriad Awaiting requested review from youknowriad

Assignees
No one assigned
Labels
[Priority] High Used to indicate top priority items that need quick attention
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Text Block: Sometimes the cursor gets moved behind the first character
4 participants
@tg-ephox @youknowriad @karmatosed @anna-harrison