draft-js icon indicating copy to clipboard operation
draft-js copied to clipboard

Clicking on styling button steals focus from editor, sometimes doesn't apply style to document

Open YOO629 opened this issue 7 years ago • 25 comments

Hi, I'm trying to implement a fairly simple editor with a couple of buttons for bolding, italicizing, underlining, etc.

Reading through the docs for RichUtils, it looks like I need to use RichUtils.toggleInlineStyle() if I want to toggle, for example, bold styling on the editor text -- so that's what I'm using.

The current behavior is that if I type in text in my editor and then click on the button that I have set up to toggleBold on click, unless I have text actually highlighted at the time of the click, the document's styling isn't changed at all. In the case where I have text highlighted at the time of the click, the button toggles bold styling for the selected text (as expected).

What I expect to happen is that if I click my bold button and don't have anything highlighted, it should apply bold styling to everything that I type from that point onwards until I toggle it off again. It should also return the focus back to editor, preferably to the place I last had my cursor before I clicked the button. Is there a clean way to achieve this?

It looks like the click on the button is what's causing the issue, probably because it takes focus from the editor?

fiddle: https://jsfiddle.net/bc76kw69/

YOO629 avatar Oct 01 '16 18:10 YOO629

I'm having similar issue. It seems that onMouseDown (with preventDefault) preserves the focus selection , but onClick have a different behavior. Style toggle works fine when button is clicked several times but it removes the focus on the selected node. a similar bug reported here #485 ¿bug or feature?

michelson avatar Oct 04 '16 01:10 michelson

I think, Inline style always applies based on selection of editorstate.you can try to toggle block style see if it works

mzbac avatar Oct 04 '16 21:10 mzbac

When editor gets the focus --- either by this.refs.editor.focus() or by the user clicking on the editor --- it seems to clear the inline style that is already applied. This doesn't happen when a range of text is selected. It happens only when selection.isCollapsed() is true.

This can be replicated even with the editor at https://facebook.github.io/draft-js/

  1. Type 'Hello' in the editor
  2. Either hit ctrl+b or click on the "Bold" item. The "Bold" item turns blue. This is the expected behaviour.
  3. Click on the text above the editor. Blue turns gray. This is totally unexpected!

Try repeating these steps by selecting 'Hello' before clicking the "Bold" item. Blue remains Blue!

It seems like there is an error with the way focus is handled when a range is not selected. This seems to be the section of the code that handles focus request on the editor.

https://github.com/facebook/draft-js/blob/master/src/component/base/DraftEditor.react.js

/**
   * Used via `this.focus()`.
   *
   * Force focus back onto the editor node.
   *
   * Forcing focus causes the browser to scroll to the top of the editor, which
   * may be undesirable when the editor is taller than the viewport. To solve
   * this, either use a specified scroll position (in cases like `cut` behavior
   * where it should be restored to a known position) or store the current
   * scroll state and put it back in place after focus has been forced.
   */
  _focus(scrollPosition?: DraftScrollPosition): void {
    const {editorState} = this.props;
    const alreadyHasFocus = editorState.getSelection().getHasFocus();
    const editorNode = ReactDOM.findDOMNode(this.refs.editor);

    const scrollParent = Style.getScrollParent(editorNode);
    const {x, y} = scrollPosition || getScrollPosition(scrollParent);

    editorNode.focus();
    if (scrollParent === window) {
      window.scrollTo(x, y);
    } else {
      Scroll.setTop(scrollParent, y);
    }

    // On Chrome and Safari, calling focus on contenteditable focuses the
    // cursor at the first character. This is something you don't expect when
    // you're clicking on an input element but not directly on a character.
    // Put the cursor back where it was before the blur.
    if (!alreadyHasFocus) {
      this.update(
        EditorState.forceSelection(
          editorState,
          editorState.getSelection()
        )
      );
    }
  }

I have tried a few basic checks to see if editorState.getSelection() has any invalid values when a range isn't selected... like whether editorState.getSelection().getStartOffset() gives the correct offset. That seems okay.

Any ideas about what the issue could be?

sandzone avatar Oct 08 '16 02:10 sandzone

any solution here ?

martinezguillaume avatar May 18 '17 12:05 martinezguillaume

Hello @martinezguillaume I have apparently found the reason why the example of DraftJS works and why it all of a sudden breaks everywhere else when you try to implement the same. Please avoid forcing the focus using their exposed focus() method when putting in any kind of inline styling or block styling, that will give you absolutely unpredictable results as that method is asynchronous and that greatly matters when it comes to changing of your editor state. Now for the fix, what you can do is create a wrapper element using a <div>button inside<div> and on this div, use the onMouseDown attribute to call the toggle function but before doing that capture the event and preventDefault to it. This is prevent the editor from losing focus and will work as expected. Here is an example code of the wrapper that I wrote and works totally fine---->

import React from 'react';
import PropTypes from 'prop-types';

const ImprovisedButton = props => (
  <div
    onMouseDown={(event) => {
      event.preventDefault();
      props.toggleFunction();
    }}
  >
    {props.children}
  </div>
);

ImprovisedButton.propTypes = {
  toggleFunction: PropTypes.func.isRequired,
  children: PropTypes.element.isRequired,
};
export default ImprovisedButton;

Pipe-Runner avatar May 20 '17 22:05 Pipe-Runner

Thanks @AakashMallik, works for me!

mettin avatar May 22 '17 22:05 mettin

My pleasure

On 23 May 2017 3:31 a.m., "Mettin Parzinski" [email protected] wrote:

Thanks @AakashMallik https://github.com/aakashmallik, works for me!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebook/draft-js/issues/696#issuecomment-303231969, or mute the thread https://github.com/notifications/unsubscribe-auth/ASpTUndc1dkCNlNPeCmoProV3SUWJbj8ks5r8gW-gaJpZM4KL0xU .

Pipe-Runner avatar May 23 '17 02:05 Pipe-Runner

using onMouseDown worked fine for mouse events, but not with keyboard events. None of the keyboard events worked for me.

However, if I set the focus to the editor explicitly in my event handler, and then wrap the call to the togglefunction in a setTimeout(fn, 0), it works fine for keyboard events also.

zeorin avatar Jun 07 '17 21:06 zeorin

I'll look into it... i really dont follow to be honest coz we have developed a full fledged editor using mouse down so far and everything is working as expected... but i'll give u some update soon...

On 8 Jun 2017 3:29 a.m., "Xandor Schiefer" [email protected] wrote:

using onMouseDown worked fine for mouse events, but not with keyboard events. None of the keyboard events worked for me.

However, if I set the focus to the editor explicitly in my event handler, and then wrap the call to the togglefunction in a setTimeout(fn, 0), it works fine for keyboard events also.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebook/draft-js/issues/696#issuecomment-306937606, or mute the thread https://github.com/notifications/unsubscribe-auth/ASpTUgiRpyHJ_uH7mBbQdT2RjgBYx4Daks5sBx1GgaJpZM4KL0xU .

Pipe-Runner avatar Jun 08 '17 03:06 Pipe-Runner

@zeorin Dude, I am currently working in a startup and I am a little bit caught up in work.... But I assure u it is possible, I wish I could have shown you the code ri8 now, but I guess I'll have to wait till this internship is over to smuggle the code out to you.... LOL

Pipe-Runner avatar Jun 09 '17 09:06 Pipe-Runner

Now for the fix, what you can do is create a wrapper element using a <div>button inside<div> and on this div, use the onMouseDown attribute to call the toggle function but before doing that capture the event and preventDefault to it.

It works well as style buttons displayed directly on the toolbar. I'm working on a color picker which opens a popover containing color items. However, at this time I have no idea how to prevent editor from loosing focus when I open the color picker.

lcc19941214 avatar Sep 19 '17 07:09 lcc19941214

@icc19941214 I am experiencing the same issue as you I am using npm color-picker as my component for colors and it seems that whenever I click on a color I lose focus. e.preventDefault() wont work because it does not know what it is :(

tylerkrett avatar Sep 21 '17 12:09 tylerkrett

@tylerkrett Here is what I did.

Add a onMouseDown event to my colorPicker button. Then add onMouseDown event to every color item as follows. The point is to make every onMousedDown event to call the preventDefault method, just as what I did to toolbar buttons.

...
onTogglePopover = (e) => {
  e.preventDefault();
}

handleApplyColor = (color, e) => {
   e.preventDefault();
}
...
<div className="RichEditor-toolbar__color-picker RichEditor-toolbar-button__wrapped">
  <span
    onMouseDown={this.onTogglePopover}>
    color picker
  </span>
</div>
...
{COLORS_MAP.map(v => (
  <span
    onMouseDown={this.handleApplyColor.bind(this, v.style)}
  />
))}
...

It works well. Hope it helps you in some way.

lcc19941214 avatar Sep 21 '17 13:09 lcc19941214

preventDefault won't work for multi-step actions like "add link" where user will have to type a link to another input field and focus will be lost anyway. I've solved it by focusing the selection before applying the result of multi-step action.

nextEditorState = EditorState.acceptSelection(
    editorState,
    editorState.getSelection().merge({
        hasFocus: true
    })
);

tomaskikutis avatar Apr 17 '18 13:04 tomaskikutis

There are a bunch of hacks and workarounds mentioned here, but is there any plan/timeline for fixing this issue?

robbyemmert avatar Jun 01 '18 02:06 robbyemmert

The docs should at least be updated to reflect the recommended workaround. I'm experiencing this issue following the basic introductory tutorial. Frustrating running into bugs (with no fix in sight) while trying to learn how the library is supposed to work.

UPDATE: In case it's useful to know, adding stopPropagation() and preventDefault() and using the mousedown event handler worked for me. Is that how this is supposed to work? If so, it might be a good idea to update the tutorial at https://draftjs.org/docs/quickstart-rich-styling.html to reflect that.

robbyemmert avatar Jun 01 '18 02:06 robbyemmert

Thought I'd post this here since I also struggled with this issue. tomaskikutis had the right idea about dealing with more complex UI controls that go beyond a simple click, the problem I had though was it didn't work for collapsed selections. I found the the code below to work as expected.

applyStyle = (style, editorState) => {
    const editorStateFocused = EditorState.forceSelection(
      editorState,
      editorState.getSelection(),
    );

    return RichUtils.toggleInlineStyle(editorStateFocused, style);
};

lucyluu22 avatar Oct 06 '18 13:10 lucyluu22

using onMouseDown worked fine for mouse events, but not with keyboard events. None of the keyboard events worked for me.

However, if I set the focus to the editor explicitly in my event handler, and then wrap the call to the togglefunction in a setTimeout(fn, 0), it works fine for keyboard events also.

Thanks maaan!! This saved my ass! :)

jblazevicClover avatar Nov 13 '19 13:11 jblazevicClover

Thought I'd post this here since I also struggled with this issue. tomaskikutis had the right idea about dealing with more complex UI controls that go beyond a simple click, the problem I had though was it didn't work for collapsed selections. I found the the code below to work as expected.

applyStyle = (style, editorState) => {
    const editorStateFocused = EditorState.forceSelection(
      editorState,
      editorState.getSelection(),
    );

    return RichUtils.toggleInlineStyle(editorStateFocused, style);
};

This solution worked for me and allowed me to use onClick with a real button element (no onClickCapture or preventDefault needed) instead of onMouseDown on a div, which would've had severe accessibility issues (no keyboard support or screen reader support without additional code).

justinryder avatar Mar 17 '20 21:03 justinryder

One solution that seems to work for me on React is as follows:

  1. Keep a reference to the Editor - for a functional component use the hook useRef
  2. Replace the 'onClick' event with 'onMouseDown'
  3. In the event handler, preventDefault and keep the focus on the Editor with ref.current.focus()

AarOmoPer avatar Aug 29 '20 02:08 AarOmoPer

onMouseDown and focus() and all other workarounds seem fine. However, when you have other UI buttons with your editor, such as Select, Input, etc. will you have to handle all mousedown event? I don't think it's a good way. P.S. for now, I may use readonly to deal with this situation

Marckon avatar Dec 04 '20 06:12 Marckon

preventDefault won't work for multi-step actions like "add link" where user will have to type a link to another input field and focus will be lost anyway. I've solved it by focusing the selection before applying the result of multi-step action.

nextEditorState = EditorState.acceptSelection(
    editorState,
    editorState.getSelection().merge({
        hasFocus: true
    })
);

could you give me an example of how this works for you? I have the same problem which I need to keep the selection text background style while adding the link for the selection like this example https://codepen.io/Kiwka/pen/ZLvPeO?editors=1010

rickycai-2020 avatar Jan 18 '21 12:01 rickycai-2020

applyStyle

do you have example code?

markstock7 avatar Mar 02 '21 08:03 markstock7

preventDefault won't work for multi-step actions like "add link" where user will have to type a link to another input field and focus will be lost anyway. I've solved it by focusing the selection before applying the result of multi-step action.

nextEditorState = EditorState.acceptSelection(
    editorState,
    editorState.getSelection().merge({
        hasFocus: true
    })
);

could you give me an example of how this works for you? I have the same problem which I need to keep the selection text background style while adding the link for the selection like this example https://codepen.io/Kiwka/pen/ZLvPeO?editors=1010

It would be great/appreciable if a example is posted here,

Could you please post a example here how to handle the select/input elements, i have same scenario to handle..

abduleichiba avatar Nov 23 '21 10:11 abduleichiba

Hello @martinezguillaume I have apparently found the reason why the example of DraftJS works and why it all of a sudden breaks everywhere else when you try to implement the same. Please avoid forcing the focus using their exposed focus() method when putting in any kind of inline styling or block styling, that will give you absolutely unpredictable results as that method is asynchronous and that greatly matters when it comes to changing of your editor state. Now for the fix, what you can do is create a wrapper element using a <div>button inside<div> and on this div, use the onMouseDown attribute to call the toggle function but before doing that capture the event and preventDefault to it. This is prevent the editor from losing focus and will work as expected. Here is an example code of the wrapper that I wrote and works totally fine---->

import React from 'react';
import PropTypes from 'prop-types';

const ImprovisedButton = props => (
  <div
    onMouseDown={(event) => {
      event.preventDefault();
      props.toggleFunction();
    }}
  >
    {props.children}
  </div>
);

ImprovisedButton.propTypes = {
  toggleFunction: PropTypes.func.isRequired,
  children: PropTypes.element.isRequired,
};
export default ImprovisedButton;

Thannnnnnnnnnk you siir!

MLyousfi avatar Apr 14 '22 01:04 MLyousfi