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

ENH Refactor Element to use mutation hook #1174

Closed

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented Apr 22, 2024

Issue #1164

Notes:

  • I haven't implemented the AC "(refactor) Triggering actions on state change" - I've made a comment about it here - I don't intend to implement it though if someone is able to demonstrate a non-convoluted way to implement an alternative I'm happy to refactor
  • I haven't implemented the AC "Passing down callbacks in context (note that context is required to pass props to the buttons because they're added via js injector in registerTransforms.js)" - I've made a comment about it here. I do not intend to implement this AC
  • I've removed some jest tests on SaveAction-test.js and PublishAction-test.js as there is simply now less logic and props passed via context in SaveAction and PublishAction. I choose not to add in equivalent tests in Element-test.js adding in "child Actions" would require doing stuff with injector to replicate what registerTransforms.js does now, and also because these functionality is already covered by behat tests in edit-block-element.feature (save button) and publish-block-element.feature (publish button)

Update:
Don't merge this, just keep this here for reference

Got extremely stuck where somehow a second Element was being created though not actually used. Looks like it was being created as part of handleFormSchemaSubmitResponse() where state hooks were being set, though possibly something wasn't wrapped in useEffect() things got out of sync. This was detected because there was failing behat test where the page was submitted with invalid data, then immediately inline save the element, which should also fail and pop open the inline form. However the inline form wasn't popping open because the duplicated element was receiving the new props

@emteknetnz emteknetnz force-pushed the pulls/5/refactor-graphql branch from 3028488 to d8e5382 Compare April 22, 2024 02:04
@emteknetnz emteknetnz changed the title ENH Refactor Element.js to use mutation hook ENH Refactor Element to use mutation hook Apr 22, 2024
@emteknetnz emteknetnz force-pushed the pulls/5/refactor-graphql branch 4 times, most recently from c0a0206 to 57ce186 Compare April 22, 2024 05:55
@emteknetnz emteknetnz force-pushed the pulls/5/refactor-graphql branch from 57ce186 to de884cc Compare April 23, 2024 04:22
@@ -128,6 +133,7 @@ ElementEditor.propTypes = {
}),
};

const filteredElementFormStates = {};
Copy link
Member Author

Choose a reason for hiding this comment

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

I tried add this to the "graphql2" PR however it caused the issue where submitting a page with an invalid elemental field and then inline saving did not pop open the preview

Don't try and make this code work, it's not necessary

@emteknetnz emteknetnz closed this Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant