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

Fix remove files #496

Conversation

jeffcaulfield-morphsites

This is a cherry-pick of the changes from #291 (without all the reformatting)—in turn derived from #108—to make the "delete" button work on File fields in flexible layout sections.

@wize-wiz
Copy link

wize-wiz commented Jan 16, 2024

@jeffcaulfield-morphsites I have a few tiny questions ;)

Either I'm missing some files to look at or I'm missing the point of all these changes.

  • I can't find the use of trait AvailableFieldsAsFieldCollection anywhere?
  • The new FieldCollection is only used in AvailableFieldsAsFieldCollection::availableFields but this trait is again, not used anywhere?

I'm not quite sure what the intend was here. I was searching for several methods to obtain a flat collection of fields fixing the dependsOn problem. I have to say honestly that FieldCollection::findFieldByAttribute and FieldCollection::flattenGroups are quite messy.

I would indeed applaud for a custom FieldCollection to flatten the layouts to obtain all fields.

@jeffcaulfield-morphsites
Copy link
Author

jeffcaulfield-morphsites commented Jan 16, 2024

@wize-wiz As mentioned in my original comment, this is a cherry-pick of the changes from an issue opened a long time ago to fix a specific problem related to File/Image fields in the context of a flexible content section. I tried to bring the changes forward more or less verbatim, only without the PSR reformatting "noise" of #291.

I'll have to look at this again later an see if I missed something during the cherry-pick operation. The changes definitely seemed to fix the "delete" problem on an earlier version of this component…

cc @flxsource

@jeffcaulfield-morphsites
Copy link
Author

My apologies: I thought I'd tested this change but apparently I was mistaken. I'll withdraw this pull request and try again later.

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.

2 participants