Hey there,

First, let me congratulate all the maintainers and contributors for the work on this D8 port, nice job.
Just one thing that I find a bit problematic is that the module is dependent on field_ui, which is really not good for production environment (perfs & security).
You could consider splitting some of the code into a conditional_fields_ui sub module.

I know it may be a lot of work, and sorry I have not a minute to help with a first patch, but I think at the end it would really worth it.

Best regards,
Renaud

CommentFileSizeAuthor
#12 conditional-fields.patch476 bytesnigelcunningham
#9 field_ui_dependency-2842914.patch266 bytesstephaneq
#8 conditional-fields_no-fields-ui_2842914_8.patch252 bytespancho
#5 2842914-alpha4.patch418 bytesovanes
#4 2842914-4.patch252 bytesrodrigoaguilera

Comments

renaudcuny created an issue. See original summary.

OlgaRabodzei’s picture

OlgaRabodzei commented

Hi, Renaud!

Thank You for the feedback. I'll add this in TODO list and will try to check the possibility to remove the dependency.

Regards.

sinn’s picture

sinn commented

Would be very useful because field_ui is disabled on production sites usually.

rodrigoaguilera’s picture

rodrigoaguilera
Primary language Spanish
Location Madrid
commented
Status: Active » Needs review
StatusFileSize
new 2842914-4.patch252 bytes

I checked that the functionality is not broken without the dependency so this patch is useful so we don't have field_ui enabled.

I didn't check the code of this module so I don't know if the dependency is really needed.

ovanes’s picture

ovanes commented
StatusFileSize
new 2842914-alpha4.patch418 bytes

Adding a patch for composer-based projects that use the latest release (alpha4), This shouldn't be applied to HEAD of course.

pwolanin’s picture

pwolanin commented

I agree strongly with the premise of this issue - no contrib module should require field_ui to be on to function.

k3vin_nl’s picture

k3vin_nl commented

Patch from #5 seems to work!

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
StatusFileSize
new conditional-fields_no-fields-ui_2842914_8.patch252 bytes

Here's a patch that should correctly apply. Let's see if it tests green.

stephaneq’s picture

stephaneq
Primary language French
Location Paris 🇫🇷
commented
StatusFileSize
new field_ui_dependency-2842914.patch266 bytes

Created new patch after the change introduced by #2998570: Apply new {project}:{module} format for dependencies in info.yml.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented

Yay, thanks!

pwolanin’s picture

pwolanin commented
Status: Needs review » Reviewed & tested by the community

FYI, composer won't apply the patch from #9 to the alpha5 due to drupal.org packaging info

This should be committed.

nigelcunningham’s picture

nigelcunningham commented
StatusFileSize
new conditional-fields.patch476 bytes

Here's a version that can be used with composer until the patch in #9 is applied.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Thanks all.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.