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

Sibiling Inserter doesn't insert in Firefox and Safari #7508

Closed
bph opened this Issue Jun 23, 2018 · 5 comments

Comments

Projects
None yet
4 participants
@bph
Contributor

bph commented Jun 23, 2018

Describe the bug
When clicking the sibling inserter between blocks it doesn't insert a new block but focuses on the block again.
Only in Firefox.

To Reproduce
Steps to reproduce the behavior:

  1. Open Firefox Browser
  2. Add to Twitter Embeds and a paragraph
  3. click on sibiling inserter to add a block

Bonus bug, if you add a paragraph at the end of the page you are not able to add any other blocks to your post. (Already reported issue #7006)

Expected behavior
Insert a new block

Screenshots
http://recordit.co/x2XzEuyoLc

Desktop (please complete the following information):

  • OS: os X 10.11.6
  • Browser: Firefox Developer Edition: 60.0b15 (64-bit)
  • Browser: Firefox Quantum 58.0.2 (64-bit)
@tofumatt

This comment has been minimized.

Member

tofumatt commented Jun 23, 2018

I can reproduce this in Firefox Developer Edition 62.0b2 (64-bit) on MacOS 10.13.5.

Looking into it now.

@tofumatt tofumatt self-assigned this Jun 23, 2018

@tofumatt tofumatt added this to the 3.2 milestone Jun 23, 2018

@tofumatt

This comment has been minimized.

Member

tofumatt commented Jun 23, 2018

I'm just poking around right now but disabling https://github.com/WordPress/gutenberg/blob/master/components/tooltip/index.js#L131 fixed it in Firefox.

EDIT: That's not entirely true. Disabling that and forcing the inserters to always be visible:

// editor/components/block-list/insertion-point.js
render() {
	const { isInserterFocused } = this.state;
	const { showInsertionPoint, showInserter } = this.props;

	return (
		<div className="editor-block-list__insertion-point">
			{ true && <div className="editor-block-list__insertion-point-indicator" /> }
			{ true && (
				<div className={ classnames( 'editor-block-list__insertion-point-inserter', { 'is-visible': true } ) }>
					<IconButton
						icon="insert"
						className="editor-block-list__insertion-point-button"
						onClick={ this.onClick }
						label={ __( 'Insert block' ) }
						onFocus={ this.onFocusInserter }
						onBlur={ this.onBlurInserter }
					/>
				</div>
			) }
		</div>
	);
}

Makes it work. 🤔

@tofumatt

This comment has been minimized.

Member

tofumatt commented Jun 23, 2018

FYI: git bisect shows 7dff9bd is the culprit here (I suspected).

@tofumatt

This comment has been minimized.

Member

tofumatt commented Jun 23, 2018

Given forcing the inserter to stay open fixes the bug… I wonder if the bit in the commit about "Hide the outline when we click the in between inserter" is why this is happening, but I'll have to look more later.

@gziolo gziolo added the [Type] Bug label Jun 25, 2018

@gziolo

This comment has been minimized.

Member

gziolo commented Jun 25, 2018

The same issue happens in the latest Safari on macOS.

@gziolo gziolo changed the title from Sibiling Inserter doesn't insert in Firefox to Sibiling Inserter doesn't insert in Firefox and Safari Jun 25, 2018

jasmussen added a commit that referenced this issue Jun 25, 2018

Try another approach to fixing the sibling inserter in Firefox
This PR partially fixes #7508. It is an alternate version of #7525, though much the same.

In #7220 we introduced a new behavior for the sibling inserter, which requires a click on the plus in the center as opposed to just clicking between two blocks.

Turns out it was sort of a race condition between onClick and onMouseDown, the latter which fires first. So in a way, the Firefox and Safari behavior of selecting the block (which is selected onMouseDown) as opposed to clicking the sibling inserter was the correct one.

This PR "fixes" it by also making the sibling inserter use onMouseDown. But in addition to this, it uses onClick as well, so it's still keyboardable.

The net effect is that both work, with the added benefit that in Firefox and Safari, the block that you're hovering isn't briefly "selected" when you're clicking.

jasmussen added a commit that referenced this issue Jun 26, 2018

Try another approach to fixing the sibling inserter in Firefox (#7530)
* Try another approach to fixing the sibling inserter in Firefox

This PR partially fixes #7508. It is an alternate version of #7525, though much the same.

In #7220 we introduced a new behavior for the sibling inserter, which requires a click on the plus in the center as opposed to just clicking between two blocks.

Turns out it was sort of a race condition between onClick and onMouseDown, the latter which fires first. So in a way, the Firefox and Safari behavior of selecting the block (which is selected onMouseDown) as opposed to clicking the sibling inserter was the correct one.

This PR "fixes" it by also making the sibling inserter use onMouseDown. But in addition to this, it uses onClick as well, so it's still keyboardable.

The net effect is that both work, with the added benefit that in Firefox and Safari, the block that you're hovering isn't briefly "selected" when you're clicking.

* Use the onClick handler for mouseDown

* Fix issues in Firefox and Safari

Turns out this issue has been present in some hidden form for a long time, and _partially fixed_ in Chrome. But there has always been a race condition, it looks like, between onClick, onFocus and onMouseDown.

This commit simply adds the "onFocusInserter" action to the onMouseDown event as well, which seems to solve the issue.

You still briefly see the block being selected and the toolbar appearing in Firefox and Safari, but at least this can work as a hotfix pending further investigation or improvements.

* Add comment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment