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 3 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
    4 days ago
    #527215
  • ๐Ÿ‡ง๐Ÿ‡ช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
    4 days ago
    #527220
Production build 0.71.5 2024