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

Setting/unsetting alignment causes Edit component to unmount/mount #35949

Open
senadir opened this issue Oct 26, 2021 · 4 comments
Open

Setting/unsetting alignment causes Edit component to unmount/mount #35949

senadir opened this issue Oct 26, 2021 · 4 comments
Labels
[Feature] Block API API that allows to express the block paradigm. [Type] Bug An existing feature does not function as intended

Comments

@senadir
Copy link
Contributor

senadir commented Oct 26, 2021

Description

When you change a block alignment from none to something else or the reverse, the Edit function gets unmounted and mounted again. This causes any state/refs/effects on the function to get lost.

Step-by-step reproduction instructions

I can't think of any core block that has both alignment and uses user-controllred state so I tested in two ways:

HTML block

  • Add alignment options to HTML block here.
  • Insert HTML block, type some HTML.
  • Switch to preview mode.
  • Change alignment.
  • Notice that the block switched back to HTML mode.

Any other block with alignment option.

  • Add this code to the Edit component:
useEffect( () => {
	console.log( 'runs on initial mount only' );
}, [] );
  • Insert said block, you should see the console above.
  • Change alignment from none to something else, you should see it run again.
  • Unset alignment, the console should run again.

Screenshots, screen recording, code snippet

switching.mov
cart-block.mov

Environment info

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@senadir senadir added [Type] Bug An existing feature does not function as intended [Feature] Block API API that allows to express the block paradigm. labels Oct 26, 2021
@ntsekouras
Copy link
Contributor

I'm not sure if we can avoid this as we want an extra wrapper for the alignments to work properly.. 🤔 --cc @ellatrix @youknowriad

@youknowriad
Copy link
Contributor

I think this can't be avoided for classic themes but I think that for themes with layout support we might be able to remove that extra div and potentially rely on the align classes like the frontend. That said, this kind of work is always tricky but definitely worth a try.

@senadir
Copy link
Contributor Author

senadir commented Nov 1, 2021

After doing some digging around, the default value of none also plays into account here, having no alignment is set to values like: "none", "", and undefinded. When I attempted to unify them all to "none", I managed to reduce the unmounting here, but I believe the code that you linked to @ntsekouras is the reason.

@ntsekouras
Copy link
Contributor

Actually when unsetting or setting to none the attribute value is undefined and is not serialized in the markup - we don't end up with align: "none" and we shouldn't 😄 . The empty string case I think it's something with some old behavior and might be not relevant now - I haven't looked into it though..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants