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

Block Editor: Simplify Selection Reducer #17467

Merged
merged 3 commits into from
Sep 25, 2019
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Add comment when reducer resets state by default
  • Loading branch information
ellatrix committed Sep 25, 2019

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
commit 243e24532e8d81527429f5ae12e81afc64966819
2 changes: 2 additions & 0 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
@@ -1092,6 +1092,8 @@ export function initialPosition( state, action ) {
} else if ( action.type === 'REMOVE_BLOCKS' ) {
return state;
}

// Reset the state by default (for any action not handled).
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This reducer is a little unconventional, in that its default behaviour isn't to return state, but rather to implicitly return undefined. Is this the best way to proceed? Seems like it could increase the likelihood of mistakes in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

How so? The reducer should store the initial position temporarily, until another action occurs. The only exception is REMOVE_BLOCKS. As mentioned above in the doc, the reducer should eventually be removed in favour of setting selection directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is more or less the same observation as in #17482 (comment)

I'll defer to your judgment, as I don't want to block this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I'll add a comment.


export function blocksMode( state = {}, action ) {