Contrib.social

hook_field_*_third_party_settings_form() is called for every field on the manage display page

Open on Drupal.org →
Created on 28 December 2023, 8 months ago
Updated 21 February 2024, 7 months ago

Problem/Motivation

The hook_field_formatter_third_party_settings_form() and hook_field_widget_third_party_settings_form() hooks are invoked for every field on the Manage Display page, even though none of its output is visible until a cog is opened.

This is because in order to know whether to show a settings cog or not, we need to know whether there are settings to edit, and we can't know that without invoking the hook:

        $third_party_settings_form = $this->thirdPartySettingsForm($plugin, $field_definition, $form, $form_state);
        if (!empty($settings_form) || !empty($third_party_settings_form)) {

However, this could be optimised: if $settings_form is non-empty, then there is no need to invoke the hook, as we know the cog should be shown anyway.

Steps to reproduce

Proposed resolution

Rework the logic in EntityDisplayFormBase so that the hook is only invoked for a field if $settings_form is empty.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

📌 Task
Status

Needs work

Version

11.0 🔥

Component
Field  →

Last updated 1 day ago

Created by

🇬🇧United Kingdom joachim

Live updates comments and jobs are added and updated live.
  • Novice

    It would make a good project for someone who is new to the Drupal contribution process. It's preferred over Newbie.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @joachim
  • First commit to issue fork.
  • Merge request !6284Issue #3411219 by shivam-kumar: hook only invoked if $settings_form is empty → (Open) created by shivam-kumar
  • Pipeline finished with Failed
    8 months ago
    #81252
  • Status changed to Needs review 8 months ago
  • Comment 8 months ago →
    🇮🇳India shivam-kumar

    Optimized the code. if $settings_form is non-empty, then the hook_field_widget_third_party_settings_form() is not called. moved to needs review.

  • Pipeline finished with Failed
    8 months ago
    #81255
  • Status changed to Needs work 8 months ago
  • Comment 8 months ago →
    🇬🇧United Kingdom joachim

    Thanks for the MR.

    Needs some work on the nesting and conditions.

    You probably want something more like (rough pseudocode)

    if (empty(settings)) {
  $3rdparty = get3rdparty();
}

if (!empty(settings) || !empty($3rdparty) {
  form array
}

    empty() works with undefined variables, so it's fine to check $3rdparty like that.

  • Pipeline finished with Running
    8 months ago
    #81669
  • Status changed to Needs review 8 months ago
  • Comment 8 months ago →
    🇮🇳India shivam-kumar

    Hey thanks @joachim, Have done the changes as per suggestion. Moving it to needs review.

  • Status changed to RTBC 8 months ago
  • Comment 8 months ago →
    🇬🇧United Kingdom joachim

    LGTM.

  • Status changed to Needs review 7 months ago
  • Comment 7 months ago →
    🇳🇿New Zealand quietone New Zealand

    What nice change.

    I'm triaging RTBC issues → . I read the IS, the comments and the MR. I didn't find any unanswered questions.

    The comment in the MR was not wrapped correctly. I fixed that by rewording the comment. Setting to needs review for that to be checked.

  • Status changed to Needs work 7 months ago
  • Comment 7 months ago →
    🇺🇸United States smustgrave

    For the thread suggestion.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024