FIX Allow publishing non-inline-editable blocks #1312
Conversation
GuySartorelli
force-pushed
the
pulls/5.3/publish-noninline-block
branch
from
f3659fe to
d931f0a
Compare
| @@ -165,12 +167,12 @@ const Element = (props) => { | |||
|
|
|||
| // Publish action | |||
| useEffect(() => { | |||
| if (formHasRendered && doPublishElement) { | |||
| if (doPublishElement) { | |||
There was a problem hiding this comment.
Seems like this bit should not be getting removed? Checking the formHasRendered is presumably still required for regular inline editable blocks?
There was a problem hiding this comment.
Oops yup this should be using formRenderedIfNeeded. FWIW it doesn't seem to be needed based on my testing and behat - but presumably it was there for a reason and should remain there for the type of element this bug doesn't affect.
| @@ -43,6 +43,8 @@ const Element = (props) => { | |||
| const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false); | |||
| const [publishBlock] = useMutation(publishBlockMutation); | |||
|
|
|||
| const formRenderedIfNeeded = () => formHasRendered || !props.type.inlineEditable; | |||
There was a problem hiding this comment.
Honestly think it's clearer to not have this new method and just update the one place it's used to check && props.type.inlineEditable
There was a problem hiding this comment.
It's used in two places (after I fix #1312 (comment)) - I'd prefer to keep it since it makes those two places clearer to me, but if you still think it's better inlined after seeing it in situ with both conditions, I'll change it.
There was a problem hiding this comment.
I have converted it from a function to a variable though. Dunno why I thought it should be a function lol
GuySartorelli
force-pushed
the
pulls/5.3/publish-noninline-block
branch
from
d931f0a to
28461ef
Compare
There was a problem hiding this comment.
I performed and updated kitchen sink CI run where there was a failing behat test with the latest PRs - https://github.com/creative-commoners/recipe-kitchen-sink/actions/runs/13148450879
That is not an existing failure - https://github.com/silverstripe/recipe-kitchen-sink/actions/runs/13148969515
Needs silverstripe/silverstripe-frameworktest#225 for CI to go green, so merge that first
Issue