Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Opt-Space no longer scrolls (Chrome OSX) #244

Closed
wants to merge 6 commits into from

Conversation

brookslyrette
Copy link
Contributor

Option + Space no longer causes scrolling. This was only happening in Chrome/OSX. This was happening in the keyDown handler not the keyPress handler.

I'm not sure if I like adding the ' ' to the editor in editOnKeyDown. Maybe I should implement this as a command?

Fixes #19

editorState.getSelection(),
' '
);
this.update(EditorState.push(editorState, contentState, 'space'));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct EditorChangeType here is 'insert-characters'. (I need to document this, but one should always stick to the EditorChangeType values for push calls.)

@hellendag
Copy link

@brookslyrette on the import I ran for your last PR, our bot pulled in changes that had already been merged, I assume because the commits were still present in this stack. Can those older changes be rebased out of this PR?

Regarding the PR itself, I'm wondering if this should be handled within getDefaultKeyBinding. It would be functionally the same as handling it in editOnKeyDown, but at least it would be with other key bindings that check OS/browser combinations.

@brookslyrette
Copy link
Contributor Author

Thanks @hellendag. I'll make the corrections and rebase the PR.

I had attempted to prevent the scrolling in getDefaultKeyBinding but since I was editing the editorState I placed it in editOnKeyDown. From what I can tell, I can't add the space to the editor state from getDefaultKeyBinding without creating a command.

I'm happy to do so if you think it's a cleaner approach. What name would be appropriate for the command?

@hellendag
Copy link

Oh of course, that's a great point. There would have to be a command string for it. Let's stick with what you have.

Were you able to determine what's actually causing this? I.e. why would Draft behave differently from a plain contenteditable?

@brookslyrette
Copy link
Contributor Author

 As far as I can tell it is because document.activeElement is not the contentEditable area. It's that the div is focused:

screen shot 2016-03-29 at 12 50 20 pm

That seemed a bit too fundamental for me to be messing around with.

@hellendag
Copy link

Hmm, though that div is the contenteditable.

That seemed a bit too fundamental for me to be messing around with.

No worries, I'll see if I can track it down. :)

@brookslyrette
Copy link
Contributor Author

Just a hunch. This only seems to happen when Option + Space is entered into an empty editor.

Not sure if it's a redraw or the placeholder causing it but I'm sure it has to do with the initial state.

@hellendag
Copy link

It happens whenever we have to preventDefault on the textInput event, i.e. when we need to render the character manually rather than allowing the native behavior. This will happen at the start of a paragraph, after an immutable entity, and other cases.

It seems that if the textInput event is prevented when the keydown is allowed, the scroll behavior occurs.

Since we need to prevent the textInput event in the case described above, I think we don't really have any choice but to prevent the keydown and perform the manual insertion as you've done here.

@hellendag
Copy link

const contentState = DraftModifier.replaceText(
editorState.getCurrentContent(),
editorState.getSelection(),
' '

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the correct character to insert here, if the goal of Opt+Space is to insert a non-breaking space? Or should this be '\u00a0'?

@hellendag
Copy link

Thanks!

@hellendag
Copy link

@facebook-github-bot import

@facebook-github-bot
Copy link

Thanks for importing. If you are an FB employee go to Phabricator to review.

@ghost ghost closed this in 358dc59 Apr 6, 2016
midas19910709 added a commit to midas19910709/draft-js that referenced this pull request Mar 30, 2022
Summary:`Option + Space` no longer causes scrolling. This was only happening in Chrome/OSX. This was happening in the keyDown handler not the keyPress handler.

I'm not sure if I like adding the ' ' to the editor in editOnKeyDown. Maybe I should implement this as a command?

Fixes #19
Closes facebookarchive/draft-js#244

Reviewed By: spicyj

Differential Revision: D3132586

fb-gh-sync-id: b94bd73094a58ce9b39528df34b899ae62366761
fbshipit-source-id: b94bd73094a58ce9b39528df34b899ae62366761
alicayan008 pushed a commit to alicayan008/draft-js that referenced this pull request Jul 4, 2023
Summary:`Option + Space` no longer causes scrolling. This was only happening in Chrome/OSX. This was happening in the keyDown handler not the keyPress handler.

I'm not sure if I like adding the ' ' to the editor in editOnKeyDown. Maybe I should implement this as a command?

Fixes #19
Closes facebookarchive/draft-js#244

Reviewed By: spicyj

Differential Revision: D3132586

fb-gh-sync-id: b94bd73094a58ce9b39528df34b899ae62366761
fbshipit-source-id: b94bd73094a58ce9b39528df34b899ae62366761
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants