Setting default values does work for boolean props

Created on 7 December 2024, 7 months ago

Overview

Given the following prop...

    expanded:
      type: boolean
      title: Expand by default
      description: Note that all accordions are expanded by default in the Experience Builder editor.
      default: false
      default_value: false
      examples: [false]

none of default, default_value, nor examples seem to work for setting a default unchecked state for a boolean prop.

This may be user-error, but even if it is, it was fairly unclear how to accomplish this. I didn't find anything in https://www.drupal.org/docs/develop/theming-drupal/using-single-director... โ†’ related to what examples are, and https://www.drupal.org/docs/develop/theming-drupal/using-single-director... โ†’ indicates that default can be used for this purpose?

Proposed resolution

Make default values work with a mechanism, or educate and document the mechanism in a more prominent place.

User interface changes

None

๐Ÿ› Bug report
Status

Active

Version

0.0

Component

Page builder

Created by

๐Ÿ‡บ๐Ÿ‡ธUnited States luke.leber Pennsylvania

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @luke.leber
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    ๐Ÿง Fixed the title! ๐Ÿ˜„

          default: false
          default_value: false
          examples: [false]
    

    XB uses the first example value as the default. You should be able to see this at /admin/config/development/configuration/single/export: export the Component config entity for this SDC and you'll explicitly see the default_value in there.

    Relevant docs: https://git.drupalcode.org/project/experience_builder/-/blob/0.x/docs/co... โ€” but that does NOT state that that is used as the default value. A slight tweak to that simple bullet would go a long way I think :)

    This means the test coverage in ui/tests/e2e/prop-types.cy.js is incomplete:

      it('Boolean', () => {
        cy.waitForElementContentInIframe('#test-bool code', 'true');
        cy.waitForElementContentNotInIframe('#test-bool code', 'false');
        cy.findByLabelText('Bool')
          .assertToggleState(true)
          .toggleToggle()
          .assertToggleState(false);
    
        cy.waitForElementContentInIframe('#test-bool code', 'false');
        cy.waitForElementContentNotInIframe('#test-bool code', 'true');
      });
    

    โ€” Any chance you could add the missing test coverage? ๐Ÿ™

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Actually, I think this is clearer still.

  • Pipeline finished with Failed
    7 months ago
    Total: 731s
    #363684
  • Pipeline finished with Failed
    7 months ago
    Total: 700s
    #363695
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States luke.leber Pennsylvania

    โ€” Any chance you could add the missing test coverage? ๐Ÿ™

    All signs point to probably not ๐Ÿ˜ญ. NW and slowly backs away.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Also, SDC allows default: โ€ฆ because it's part of JSON Schema itself, but the SDC subsystem doesn't actually use/support it. I observed this 4 months ago at #3462705-19: [META] Missing features in SDC needed for XB โ†’ .

    IOW: you're running into one of many SDC bugs/oversights we've spotted while working on XB, see ๐Ÿ“Œ [SPIKE] Comprehensive plan for integrating with SDC Active for the overview :)

  • Pipeline finished with Failed
    7 months ago
    Total: 640s
    #364297
  • Pipeline finished with Failed
    7 months ago
    Total: 771s
    #364321
  • Pipeline finished with Failed
    7 months ago
    Total: 729s
    #364348
  • Status changed to Needs work 4 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States Kristen Pol Santa Cruz, CA, USA

    I thought I already reported this a long time agoโ€ฆ hmmโ€ฆ maybe it was closed

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States Kristen Pol Santa Cruz, CA, USA

    ๐Ÿ› [PP-1] Can't toggle boolean prop back to true after changing to false Postponed was marked as duplicate but never got fixed it seems

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States Kristen Pol Santa Cruz, CA, USA

    Thanks for bumping priority :) I find that Iโ€™ll set to false and the component behavior seems fine but the UI later shows it as true even though the component still seems to behave as itโ€™s false โ€ฆ maybe thatโ€™s whatโ€™s explained but Iโ€™m on my phone on my flight to Atlanta so hard to read well :D

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • I am able to reproduce the reported bug by following the steps mentioned.

    Steps to Reproduce:

    1. Add a new code component.
    2. Add a prop with the following details:
      • Name: Expand by default
      • Type: Boolean
      • Required: False
      • Example value: False
    3. Click on "Add to component".
    {
        "errors": [
            {
                "detail": "Prop \"expandByDefault\" has invalid example value: [] String value found, but a boolean or an object is required",
                "source": {
                    "pointer": ""
                }
            },
            {
                "detail": "This value should be of the correct primitive type.",
                "source": {
                    "pointer": "props.expandByDefault.examples.0"
                }
            }
        ]
    }
    
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Thanks, that definitely seems like a blocker for ๐ŸŒฑ Milestone 1.0.0-beta1: Start creating non-throwaway sites Active then!

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Crediting @mayur-sose for confirming this is still relevant ๐Ÿ˜Š ๐Ÿ™

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    This is showing a server-side validation error, meaning the client is sending the string "false" instead of the boolean false. Seems like a pretty simple bug to fix on the client :)

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Akhil Babu Chengannur

    akhil babu โ†’ made their first commit to this issueโ€™s fork.

  • This issue has been fixed with : #3520706 ๐Ÿ› Can't add in-browser code component with checkbox prop to the library Active .
    @wim-leers we can close this ticket.

  • ๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland
  • ๐Ÿ‡ฎ๐Ÿ‡ฑIsrael heyyo Jerusalem

    I was happy to read that the boolean SDC issue has been fixed, but it looks like it didn't.
    I suppose it was marked as duplicate by mistake, thinking the issue was for code components but it is for SDC components.

  • ๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland

    Sorry about that. I think I got confused because it looks like #12 ๐Ÿ› Setting default values does work for boolean props Active was tested with code components but the original issue was for SDC ๐Ÿ˜…

  • ๐Ÿ‡ฎ๐Ÿ‡ฑIsrael heyyo Jerusalem

    I want this closed as you do, don't worry ๐Ÿ˜›

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    @mayur-sose: Why did you link to ๐Ÿ› Can't add in-browser code component with checkbox prop to the library Active ? That's completely unrelated to the bug reported here ๐Ÿ˜… That was about creating a (JavaScript)Component config entity, this is about using an SDC that is available as a component in XB, and then trying to use the widget in the component instance form on the right to populate that prop.

    Can you please test again with an SDC as described in the issue summary? ๐Ÿ™ Thanks!

  • @wim-leers, I am able to reproduce this issue where default value of `false` does not work for boolean props.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States effulgentsia
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom thoward216

    I am able to reproduce this issue with SDCs, although I am also seeing an issue whereby if I toggle a boolean field from true to false, save the changes and then refresh the page the toggle has gone back to the true state. Looking at the data saved in the database it has saved correctly there so this looks to be a UI bug.

    This can easily be seen with something like the Shoe Badge SDC and toggling the "pulse" boolean and saving the pulse effect in the preview does not happen but the toggle is showing as true.

    I wonder if this is some of the root cause to the default values not working as expected here? Or could be something separate, if so I can create a new ticket for this.

    As mentioned in #2 ๐Ÿ› Setting default values does work for boolean props Active it does look like the default value used is the first value in the examples.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom thoward216
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom thoward216
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    The phpunit test failures are trivial expectation updates.

    The back end is clearly returning (/xb/api/v0/config/component) the correct value:

            "propSources": {
                "test_bool_default_false": {
                    "required": false,
                    "jsonSchema": {
                        "type": "boolean"
                    },
                    "sourceType": "static:field_item:boolean",
                    "expression": "\u2139\ufe0eboolean\u241fvalue",
                    "default_values": {
                        "source": [
                            {
                                "value": false
                            }
                        ],
                        "resolved": false
                    }
                },
    

    Those 2 falses at the end are what's newly tested in that explicit prop shape example. Explicit test coverage for that seems excessive, but if others disagree, we could add it to ComponentInputsFormTest.

    The new e2e test then reproduces the reported bug:

          cy:command โœ˜  assert	expected **<button#edit-xb-component-props-1664b908-581b-4a4c-b485-e910cb0378b8-test-bool-default-false-value.rt-reset.rt-SwitchRoot.rt-r-size-2.rt-variant-surface.form-checkbox>** to have attribute **data-state** with the value **unchecked**, but the value was **checked**
    

    @thoward216 Can you fix the 2 PHPUnit test failures, and comment out the failing e2e test? Then we can merge this MR, and then hand this over the FE team ๐Ÿ˜Š

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    I'd swear we recently fixed this, but I can only find ๐Ÿ› "Published" checkbox is always checked even if the entity is not Active . I bet I'm missing something.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom thoward216

    I've fixed those two PHPUnit test failures, commented out the failing e2e test for now and also added a bit to the components.md - let me know if this needs expanding.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    You've commented out the entire e2e test  โ€” please comment out only the bit for test_bool_default_false ๐Ÿ™

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom thoward216

    Oh yes! That was a silly mistake, have fixed that now!

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom thoward216

    Tests are now all passing on this.

  • Pipeline finished with Skipped
    21 days ago
    #527215
  • Merge request !1175Resolve #3492368 "Boolean default false" โ†’ (Merged) created by wim leers
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Great, </code> <code>!454 added explicit test coverage for shape matching (passing), and an e2e test (failing).

    The failing test was commented out, to allow it to be worked on by somebody who knows in-depth. See #29 for the rationale.

    The !1175 MR I just created uncomments the commented out e2e test coverage @thoward216 wrote and now needs a (hopefully tiny) client-side change :)

  • Pipeline finished with Failed
    21 days ago
    #527220
  • ๐Ÿ‡ช๐Ÿ‡ธSpain penyaskito Seville ๐Ÿ’ƒ, Spain ๐Ÿ‡ช๐Ÿ‡ธ, UTC+2 ๐Ÿ‡ช๐Ÿ‡บ
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Closed ๐Ÿ› Boolean checkbox does not properly update on the UI Active as a duplicate of this one. Crediting the reporter and the person who spotted it as a duplicate.

  • First commit to issue fork.
  • Pipeline finished with Success
    10 days ago
    Total: 765s
    #536387
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

    I'd incorrectly expected this to be addressed by the de-radixification but it finally occurred to me - this is specifically for a boolean widget, not a checkbox element. The values are handled slightly differently so I wired that up and it should work fine now.

  • Pipeline finished with Success
    10 days ago
    Total: 776s
    #536395
  • Pipeline finished with Skipped
    10 days ago
    #536960
  • First commit to issue fork.
  • Pipeline finished with Skipped
    10 days ago
    #536961
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom jessebaker
  • ๐Ÿ‡ฎ๐Ÿ‡ฑIsrael heyyo Jerusalem

    Hi ,
    I'm trying to test this boolean prop, it works better, but it looks like there is still an issue, when mutiple boolean exist in the sdc.
    Changing value to boolean, will change other boolean too...
    Video attached to show this weird bug

  • ๐Ÿ‡ฎ๐Ÿ‡ฑIsrael heyyo Jerusalem
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bnjmnm Ann Arbor, MI

    I'm not able to reproduce what was reported in #46 with the "All props" component from the sdc_test_all_props module. This component has multiple booleans (one default true, one default false), though not as many as the one in the provided video

    There are also and the e2e tests using the "All props" component that would have failed had the symptoms in #46 been occurring.

    Although this does not appear reproducible with the components available within the experience builder module, we should give this a try with the component used in #46 to see what we can surface. Could @heyyo direct us to where we could find that component?

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Any chance @heyyo forgot to run npm run build? ๐Ÿคž

  • ๐Ÿ‡ฎ๐Ÿ‡ฑIsrael heyyo Jerusalem

    I really think I did rebuilt the bundle, but you never know. I'll try to recheck in 2/3 hours from now.
    The components I'm using are custom ones not available online, I may release the custom them based mainly on daisyUI and tailwind.
    But I did reproduce the same bug with 2 different SDC.

  • Pipeline finished with Success
    9 days ago
    Total: 1030s
    #537449
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Wow, nice find, both of you!

    @heyyo: could you please test @bnjmnm's follow-up MR? ๐Ÿ™

  • ๐Ÿ‡ฎ๐Ÿ‡ฑIsrael heyyo Jerusalem

    Confirmed, just tested last MT, working perfectly !
    Modifying a boolean, doesn't affect other boolean anymore.
    And exiting form, and returning doesn't set all boolean to true anymore.
    So it's working great, I will be able to play with slider container with 30 props, and maybe 25 boolean :-)

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    ๐Ÿคฃ You do you!

    Thanks so much!

  • Pipeline finished with Skipped
    9 days ago
    #537925
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
Production build 0.71.5 2024