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

Controls use different amount of space when hidden #2369

Open
rajsite opened this issue Aug 23, 2024 · 5 comments
Open

Controls use different amount of space when hidden #2369

rajsite opened this issue Aug 23, 2024 · 5 comments

Comments

@rajsite
Copy link

rajsite commented Aug 23, 2024

Describe the bug

In a horizontal layout controls that are not shown still consume space unexpectedly.

Expected behavior

I expect only controls that are shown will participate in the flex layout and space is not associated with controls that are not shown.

Steps to reproduce the issue

  1. Go to https://stackblitz.com/edit/jsonformsquerybuilder
  2. Click on "Add to query groups" to add a query group to the array. Click on "Add to queries" to a query to the sub array.
  3. Change "Property Name" to "connectionStatus".
    Notice the unexpected reserved space after the connection status value that looks incorrect:
    image
  4. Change "Property Name" to "description"
    Notice that the spacing between the description and operator field is too large and looks incorrect:
    image

The following are the schemas demonstrated. The issue appears on both the seeds I tested, the current angular seed and the current vue seed.

schema
{
  "definitions": {
    "query": {
      "type": "object",
      "properties": {
        "propertyName": {
          "type": "string",
          "enum": ["connectionStatus", "description"]
        }
      },
      "anyOf": [
        {
          "if": {
            "properties": { "propertyName": { "const": "connectionStatus" } }
          },
          "then": {
            "properties": {
              "connectionStatusOperator": {
                "type": "string",
                "enum": ["equals", "doesNotEqual"]
              },
              "connectionStatusValue": {
                "type": "string",
                "enum": ["approved", "disconnected", "connected"]
              }
            }
          }
        },
        {
          "if": {
            "properties": { "propertyName": { "const": "description" } }
          },
          "then": {
            "properties": {
              "descriptionOperator": {
                "type": "string",
                "enum": ["equals", "doesNotEqual"]
              },
              "descriptionValue": { "type": "string" }
            }
          }
        }
      ]
    },
    "queryGroup": {
      "type": "object",
      "properties": {
        "queryGroupsOperator": { "type": "string", "enum": ["AND", "OR"] },
        "queries": {
          "type": "array",
          "items": { "$ref": "#/definitions/query" }
        }
      }
    }
  },
  "type": "object",
  "properties": {
    "queryGroups": {
      "type": "array",
      "items": { "$ref": "#/definitions/queryGroup" }
    }
  }
}
uischema.json
{
  "type": "HorizontalLayout",
  "elements": [
    {
      "type": "Control",
      "label": { "text": "Query Groups", "show": true },
      "scope": "#/properties/queryGroups",
      "options": {
        "showSortButtons": true,
        "detail": {
          "type": "VerticalLayout",
          "elements": [
            {
              "type": "Control",
              "scope": "#/properties/queries",
              "options": {
                "showSortButtons": true,
                "detail": {
                  "type": "HorizontalLayout",
                  "elements": [
                    { "type": "Control", "scope": "#/properties/propertyName" },
                    {
                      "type": "Control",
                      "scope": "#/properties/connectionStatusOperator",
                      "rule": {
                        "effect": "SHOW",
                        "condition": {
                          "scope": "#/properties/propertyName",
                          "schema": { "const": "connectionStatus" }
                        }
                      }
                    },
                    {
                      "type": "Control",
                      "scope": "#/properties/connectionStatusValue",
                      "rule": {
                        "effect": "SHOW",
                        "condition": {
                          "scope": "#/properties/propertyName",
                          "schema": { "const": "connectionStatus" }
                        }
                      }
                    },
                    {
                      "type": "Control",
                      "scope": "#/properties/descriptionOperator",
                      "rule": {
                        "effect": "SHOW",
                        "condition": {
                          "scope": "#/properties/propertyName",
                          "schema": { "const": "description" }
                        }
                      }
                    },
                    {
                      "type": "Control",
                      "scope": "#/properties/descriptionValue",
                      "rule": {
                        "effect": "SHOW",
                        "condition": {
                          "scope": "#/properties/propertyName",
                          "schema": { "const": "description" }
                        }
                      }
                    }
                  ]
                }
              }
            },
            { "type": "Control", "scope": "#/properties/queryGroupsOperator" }
          ]
        }
      }
    }
  ]
}

Screenshots

See above

Which Version of JSON Forms are you using?

v3.3.0

Package

Angular Material Renderers

Additional context

No response

@lucas-koehler
Copy link
Contributor

Thanks for the report! This certainly does not look intended.
Note for development: The same problem might also occur for other renderer sets.

@lucas-koehler
Copy link
Contributor

I had a short synchronization with the team on this and it turned out it is generally on purpose that hidden fields still consume space to avoid layout shifts:

Usually, we use flex-box and create an item for each child, regardless of whether it is rendered or not. The children do not know what kind of layout they rendered in.
If you want to change this, you have to implement custom renderers:

  • Either a custom layout that analyzes the children and does not generate an “item” for each control, or
  • A custom layout that does not generate any items at all and custom controls that each generate an “item” when rendered

This was deliberately implemented like this at the time because there were use cases that required that layouts do not shift when controls are hidden or shown.

We know that your case is also valid and there is no one size fits all solution. However, we do not intend to change the general behavior at this moment.

Nevertheless, your screenshots show that the "hidden" items use less space than the visible ones. This causes layout shifts again once a hidden item becomes visible again. Thus, I'll rename the issue accordingly

@lucas-koehler lucas-koehler changed the title Hidden controls still take space in layout Controls use different amount of space when hidden Sep 4, 2024
@rajsite
Copy link
Author

rajsite commented Sep 4, 2024

I had a short synchronization with the team on this and it turned out it is generally on purpose that hidden fields still consume space to avoid layout shifts: [...] We know that your case is also valid and there is no one size fits all solution. However, we do not intend to change the general behavior at this moment.

I can appreciate that intention. My understanding is that layouts can take options. Would having that as an option on vertical / horizontal layouts be a possible direction or would you expect needing completely custom renderers? Something like reserveSpaceForHiddenItems: <boolean> / <maybe_even_a_schema_matcher>?

Edit: I guess an alternate api idea would be additional effect type like ABSENT

@lucas-koehler
Copy link
Contributor

Yes every UI Schema element can have an options property that can contain arbitrary options to be interpreted by the renderers. Thus, it should be possible to implement this in the layout renderers: They would need to filter out elements that are not visible to prevent the control renderers to render an empty element.

In this case, the option should be named in a way that its default value is the current behavior. To make clear what it actually does it could be named removeHiddenItems: <boolean>.
IMO adding a schema matcher here is a bit too specific for the general renderers.

@sdirix Do you see any concern that I overlooked with introducing this? IMO this could be a suitable solution to support both use cases. But maybe I missed something.

@sdirix
Copy link
Member

sdirix commented Sep 5, 2024

Some thoughts:

  • If it's only a layout option, then there is the downside that for all surviving elements we will do the "visible" analysis twice, once in the layout and once in the child element. For a clean solution we would therefore also need to adjust the mappings of all controls.
  • Conceptually we hand over the full rendering power to each renderer including the interpretation of what it means to be "(in)visible". By moving that into the parent we are taking this away from each renderer.
  • I'm also not sure whether we should support two options from JSON Forms' side as it means additional maintenance.

I'm interested in suggestions weighing these (and other) points against each other so we can cover all needs in some way, either built-in or via straightforward customizations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants