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

[5.x]: Entry types' field layout changes aren't saved, if the only thing that changed were the positions of layout elements #15154

Closed
mmikkel opened this issue Jun 5, 2024 · 11 comments
Labels

Comments

@mmikkel
Copy link
Contributor

mmikkel commented Jun 5, 2024

What happened?

Description

Changes to an entry type's field layout aren't actually saved, if all that was changed was field layout elements' positions:

CleanShot.2024-06-05.at.22.37.46.mp4

If other things are changed in addition to the new layout elements' position (such as adding or removing an element, changing elements' settings etc) the new layout elements positions are saved as expected.

Steps to reproduce

  1. In an entry type's edit page, move a field layout element (make no other changes)
  2. Save the entry type
  3. Confirm that the moved element is still in its previous position

Expected behavior

Saving an entry type field layout should persist layout elements' positions, even if nothing else changed.

Actual behavior

Layout elements' positions are only saved if something else was changed in the layout too.

Craft CMS version

5.2.0-beta.3

PHP version

No response

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

None

@mmikkel mmikkel added the bug label Jun 5, 2024
@brandonkelly
Copy link
Member

I’m not able to reproduce that. Are you getting any JS errors?

@mmikkel
Copy link
Contributor Author

mmikkel commented Jun 5, 2024

No JS errors, and I went back and tested w/ a clean 5.2.0-beta.3 install in a few different browsers (Chrome, Edge, Safari) – same thing.

I'm not able to reproduce at all on 5.1.9 though, so seems isolated to the 5.2 beta.

@brandonkelly
Copy link
Member

I tested on 5.1 and 5.2, but couldn’t reproduce either way. Even tried with a field layout that only contains two instances of the same field.

CleanShot.2024-06-05.at.16.43.12.mp4

Tested in Firefox, Edge, and Safari, as well as on another 5.2 install on Brad’s computer.

Did you use the craftcms/craft starter project for the fresh install, or do you have your own starter project?

@mmikkel
Copy link
Contributor Author

mmikkel commented Jun 6, 2024

That's really strange.

The fresh install is a completely stock craftcms/craft starter install, running on DDEV v1.23.1 and installed as per the official docs' Quick Start section.

Literally all I did to reproduce was to update the site to 5.2.0-beta.3 immediately after installation (5.2.0-beta.3 as 5.1.9), and then I added a single entry type with two PlainText fields.

A few other things I tried to see if it would affect anything:

  • clear-caches/all
  • restarting DDEV and OrbStack
  • changing edition from Solo to Team (and then back again)
  • tested in a InPrivate tab in Edge
  • also tested in Firefox

@mmikkel
Copy link
Contributor Author

mmikkel commented Jun 6, 2024

Here's something else that could be relevant:

I tried downgrading the fresh install from 5.2.0-beta.3 to 5.1.9 and was surprised to now being able to reproduce on that version as well. On a whim, I renamed the entry type, after which the behavior completely went away on 5.1.

I then upgraded the site back to 5.2.0-beta.3, and the behavior was immediately back. I did this two times to make sure, and it was the same each time 🤷

@i-just
Copy link
Contributor

i-just commented Jun 6, 2024

I can replicate it. Looking into it. I’ll update this issue once I know more.

@brandonkelly
Copy link
Member

brandonkelly commented Jun 6, 2024

Sorry! I was testing on installs with the 5.2 branch checked out, but I forgot to run composer update to pull in the Yii 2.0.50 update.

@i-just tracked this down to a Yii bug related to determining whether JSON columns have changed, which was exacerbated by yiisoft/yii2#19915 added in 2.0.49.

I’ve reported the bug (yiisoft/yii2#20191) and submitted a PR to fix (yiisoft/yii2#20192). In the meantime, the least-hacky way I could find to fix this in Craft was to change the nature of ArrayHelper::recursiveSort() so it skips non-associative arrays altogether. Not great but there’s no other usages of this method besides by ActiveRecord, at least in Craft/Yii. Hopefully it’s a short-lived fix.

@mmikkel
Copy link
Contributor Author

mmikkel commented Jun 6, 2024

Wow, that went a lot deeper than I thought it would 😅 Appreciate the fix Brandon & @i-just 🙌

@brandonkelly
Copy link
Member

Craft 5.2.0-beta.4 is out now with the temporary fix.

@mmikkel
Copy link
Contributor Author

mmikkel commented Jun 7, 2024

@brandonkelly Unfortunately, it looks like the temporary fix is triggering the following fatal error if the config/general.php file is using the multi-environment (i.e. not fluent) config syntax:

Fatal error: Cannot declare class yii\helpers\ArrayHelper, because the name is already in use in /var/www/html/vendor/craftcms/cms/lib/yii2/helpers/ArrayHelper.php on line 13

Plugin config files using multi-environment syntax seem fine, it's specifically a multi-environment-formatted config/general.php file that seems to trigger the error.

@brandonkelly
Copy link
Member

Doh, thanks. 5.2.0-beta.5 is out with a fix for that.

brandonkelly added a commit that referenced this issue Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants