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

Store: Remove redundant handling of inserter position #4765

Merged
merged 2 commits into from
Jan 31, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Jan 30, 2018

Blocker for #3745
Related: #4539, #3745

See: #4539 (comment)

This pull request seeks to remove redundant handling of inserter position, relevant only for the sibling inserter which was removed in #4539. It also consequently reshapes the reducer state for insertion point, which now only needs to reflect a single value reflecting whether or not the insertion point is visible.

Testing instructions:

Repeat testing instructions from #4539.

aduth added 2 commits January 30, 2018 16:34
To reflect that it merely represents a single boolean value:
visible or not visible.
@aduth aduth added the [Feature] Inserter The main way to insert blocks using the + button in the editing interface label Jan 30, 2018
@aduth aduth force-pushed the remove/redundant-sibling-inserter branch from d3b9348 to cbb5004 Compare January 31, 2018 00:19
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Code looks good and I don't see any regressions in my testing.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Good catch 👍

Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

Experience wise, this works for me.

As noted in #4669 (comment), we might still want to restore this sibling inserter and kill the side inserter, depending on feedback. But this makes for a more accurate test. Thank you.

@aduth aduth merged commit db32cf7 into master Jan 31, 2018
@aduth aduth deleted the remove/redundant-sibling-inserter branch January 31, 2018 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants