Skip to content

Experiments with React hooks#3359

Closed
swissspidy wants to merge 62 commits intodevelopfrom
try/hooks
Closed

Experiments with React hooks#3359
swissspidy wants to merge 62 commits intodevelopfrom
try/hooks

Conversation

@swissspidy
Copy link
Copy Markdown
Collaborator

@swissspidy swissspidy commented Sep 27, 2019

⚠️ Work in progress - do not merge ⚠️

Includes some experimental refactoring of dozens of components to a) make them functional ones and b) to leverage React hooks. It is largely untested, even though I haven't spot any issues so far.

This PR likely has to be split up as hooks cannot be used pre-WordPress 5.3 unless Gutenberg is active, which means they're currently guaranteed to work in the Stories editor, but not elsewhere.

@googlebot googlebot added the cla: yes Signed the Google CLA label Sep 27, 2019
getBlocksByClientId,
getAdjacentBlockClientId,
} = select( 'core/block-editor' );
const { getCurrentPage, isReordering: _isReordering } = select( 'amp/story' );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, this is a common annoying problem. What's a good name for a boolean-returning function that doesn't clash with the name of the variable holding the result of said function?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good question. To avoid it we could also just use select( 'amp/story' ).isReordering().

@swissspidy swissspidy added AMP Stories Needs Testing Issues that need to be confirmed. Enhancement New feature or improvement of an existing one labels Sep 30, 2019
@swissspidy
Copy link
Copy Markdown
Collaborator Author

Closing in favor of #3412. The block editor part can be extracted from this PR at some point in the future.

@swissspidy swissspidy closed this Oct 3, 2019
@schlessera schlessera deleted the try/hooks branch March 7, 2020 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Signed the Google CLA Enhancement New feature or improvement of an existing one Needs Testing Issues that need to be confirmed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants