Nested sitemaps

[mherwege]

This PR introduces the ability to nest sitemaps.

Using DSL, the following should become possible:

sitemap parent {
    Sitemap name=child label="Child Sitemap"
    Sitemap item=sitemapNameItem label="Child sitemap defined by string item"
}
sitemap child {
    Switch item=demoSwitch
}

This will show a link in the parent sitemap that, when clicked, will show the child sitemap.

The way this has been done is by converting the Sitemap element into a Text element at the time of rendering and including all widgets in the child sitemap as child widgets of the Text element. The Sitemap element therefore only exists in the sitemap definition, not when rendering the sitemap. This makes it possible to do this without any change to any of the sitemap UIs.

This nested sitemap can be defined in 2 ways:

• explicit by providing the sitemapname parameter
• implicit, the sitemap name will be the state of the item parameter

This supports configuration in DSL, YAML and can support UI configuration (that will require adding functionality to the sitemap editor in the UI, the core part is in this PR).

I did some quick tests with BasicUI and so far it is behaving as expected.

The UI enhancement PR to configure nested sitemaps can be found here: openhab/openhab-webui#4206

@lolodomo What do you think?

[lolodomo]

Partial review

[lolodomo]

Should rather be: priate @Nullable String sitemapName;
Then getSitemapName is properly defined.

[lolodomo]

Should rather be: public void setSitemapName(@Nullable String sitemapName) {

[lolodomo]

@lolodomo What do you think?

Why not, even if I don't find myself a real need for that.

[lolodomo]

• implicit, the sitemap name will be the state of the item parameter

You have a use case for that ?
I don't imagine in which case it could be useful..

[mherwege]

Anything can be done using the visiblity rules an putting everything inside a single sitemap. But this would allow me to create different sitemaps for, e.g. time of day, location of a person, and dynamically show or hide links to these. These 'block' of ui can then be defined in separate sitemaps, and also be reused between sitemap. It adds a layer of flexibility and reduces potential configuration duplication.
When creating sitemaps through the UI builder, the UI visualisation can get very long when you have a lot of visiblity rules in there switching parts of sitemaps on/off. Again, this allows to create separate sitemaps and link them together, keeping the UI less cluttered.
Is it a must, no. It is a nice to have.

[Copilot]

Pull request overview

This PR adds a new sitemap widget type (Sitemap) that links to and renders another sitemap “inline” by resolving it at render time into a Text widget whose children are copies of the target sitemap’s widgets. This enables “nested sitemaps” across DSL and YAML (and lays groundwork for UI-based configuration) without requiring changes in existing sitemap UIs.

Changes:

• Introduces NestedSitemap widget type (DSL/YAML/UI component mapping) and factory support to create it.
• Updates UI rendering (ItemUIRegistryImpl) to resolve NestedSitemap into a Text widget and copy target sitemap widgets as children.
• Extends validation and tests (DSL validator, YAML DTO validation/provider tests, UI registry tests) to cover the new widget.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
bundles/org.openhab.core.ui/src/test/java/org/openhab/core/ui/internal/items/ItemUIRegistryImplTest.java	Adds tests and mocks to cover nested-sitemap resolution and widget id traversal.
bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java	Resolves NestedSitemap to Text, copies widgets, and adjusts child/widget-id traversal to use resolved children.
bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/components/UIComponentSitemapProvider.java	Adds UI component → widget property mapping for sitemapName.
bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/components/UIComponentSitemapMapper.java	Maps NestedSitemap to UI component config (sitemapName).
bundles/org.openhab.core.sitemap/src/main/java/org/openhab/core/sitemap/Sitemap.java	Moves widget list accessors off Sitemap (now inherited via Parent).
bundles/org.openhab.core.sitemap/src/main/java/org/openhab/core/sitemap/Parent.java	Becomes the common interface for getWidgets()/setWidgets() across sitemap/widget parents.
bundles/org.openhab.core.sitemap/src/main/java/org/openhab/core/sitemap/NestedSitemap.java	Adds new widget interface for nested sitemaps (sitemapName).
bundles/org.openhab.core.sitemap/src/main/java/org/openhab/core/sitemap/LinkableWidget.java	Removes duplicated widget-list accessors now provided by Parent.
bundles/org.openhab.core.sitemap/src/main/java/org/openhab/core/sitemap/internal/registry/SitemapFactoryImpl.java	Registers Sitemap widget type and instantiates NestedSitemapImpl.
bundles/org.openhab.core.sitemap/src/main/java/org/openhab/core/sitemap/internal/NestedSitemapImpl.java	Implements the new NestedSitemap widget.
bundles/org.openhab.core.sitemap/src/main/java/org/openhab/core/sitemap/dto/WidgetDefinitionDTO.java	Adds sitemapName field for widget definition serialization.
bundles/org.openhab.core.model.yaml/src/test/resources/model/sitemaps/bigSitemap.yaml	Adds a YAML example widget of type Sitemap.
bundles/org.openhab.core.model.yaml/src/test/java/org/openhab/core/model/yaml/internal/sitemaps/YamlWidgetDTOTest.java	Adds YAML DTO validation test for nested sitemap widget.
bundles/org.openhab.core.model.yaml/src/test/java/org/openhab/core/model/yaml/internal/sitemaps/YamlSitemapProviderTest.java	Extends provider test expectations to include NestedSitemap loading.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/sitemaps/YamlWidgetDTO.java	Adds fields/validation rules for YAML Sitemap widget (name or item required).
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/sitemaps/YamlSitemapProvider.java	Sets sitemapName on NestedSitemap when building widgets from YAML.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/sitemaps/YamlSitemapDTO.java	Carries name field through partial widget DTO merging.
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/sitemaps/fileconverter/YamlSitemapConverter.java	Serializes NestedSitemap to YAML widget DTO (name).
bundles/org.openhab.core.model.sitemap/src/org/openhab/core/model/sitemap/validation/SitemapValidator.xtend	Adds DSL validation for nested sitemap parameters and allows sitemapname= parameter.
bundles/org.openhab.core.model.sitemap/src/org/openhab/core/model/sitemap/Sitemap.xtext	Extends DSL grammar with ModelNestedSitemap and sitemapname= parameter.
bundles/org.openhab.core.model.sitemap/src/org/openhab/core/model/sitemap/internal/fileconverter/DslSitemapConverter.java	Adds DSL serialization for NestedSitemap.
bundles/org.openhab.core.model.sitemap/src/org/openhab/core/model/sitemap/internal/DslSitemapProvider.java	Adds DSL parsing/provider support to populate sitemapName for NestedSitemap.

Comments suppressed due to low confidence (1)

bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java:740

• The widget-id traversal now uses getChildren(), but the warning logs still report sizes from the raw getWidgets() lists (sitemap.getWidgets()/((LinkableWidget)w).getWidgets()). With nested/default resolution these counts can differ, making the warning misleading. Consider logging the resolved list sizes (sitemapWidgets.size()/childWidgets.size()) instead.

                        List<Widget> sitemapWidgets = getChildren(sitemap);
                        if (widgetID < sitemapWidgets.size()) {
                            w = sitemapWidgets.get(widgetID);
                            for (int i = codingSize; i < idValue.length(); i += codingSize) {
                                int childWidgetID = Integer.parseInt(idValue.substring(i, i + codingSize));
                                List<Widget> childWidgets = getChildren((LinkableWidget) w);
                                if (childWidgetID < childWidgets.size()) {
                                    w = childWidgets.get(childWidgetID);
                                } else {
                                    logger.warn(
                                            "Widget id '{}' is invalid, index {} outside the number ({}) of widgets in the page",
                                            id, childWidgetID, ((LinkableWidget) w).getWidgets().size());
                                    w = null;
                                    break;
                                }
                            }
                        } else {
                            logger.warn(
                                    "Widget id '{}' is invalid, index {} outside the number ({}) of widgets in the sitemap",
                                    id, widgetID, sitemap.getWidgets().size());

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java:765

• The warning messages in getWidget() still report widget counts using the raw getWidgets().size() lists, but indexing is now based on getChildren(...) (which resolves Default/NestedSitemap widgets and can filter nulls). This can produce misleading diagnostics when an id is invalid. Use sitemapWidgets.size() and childWidgets.size() in the warnings to reflect the list actually used for indexing.

                        int widgetID = Integer.parseInt(idValue.substring(0, codingSize));
                        List<Widget> sitemapWidgets = getChildren(sitemap);
                        if (widgetID < sitemapWidgets.size()) {
                            w = sitemapWidgets.get(widgetID);
                            for (int i = codingSize; i < idValue.length(); i += codingSize) {
                                int childWidgetID = Integer.parseInt(idValue.substring(i, i + codingSize));
                                List<Widget> childWidgets = getChildren((LinkableWidget) w);
                                if (childWidgetID < childWidgets.size()) {
                                    w = childWidgets.get(childWidgetID);
                                } else {
                                    logger.warn(
                                            "Widget id '{}' is invalid, index {} outside the number ({}) of widgets in the page",
                                            id, childWidgetID, ((LinkableWidget) w).getWidgets().size());
                                    w = null;
                                    break;
                                }
                            }
                        } else {
                            logger.warn(
                                    "Widget id '{}' is invalid, index {} outside the number ({}) of widgets in the sitemap",
                                    id, widgetID, sitemap.getWidgets().size());
                        }

[Copilot]

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java:772

• The warning logs for invalid widget ids use ((LinkableWidget) w).getWidgets().size() / sitemap.getWidgets().size(), but traversal now uses getChildren(...) (which can include resolved Default widgets, dynamic Group children, and nested sitemap expansion). This can produce misleading counts in logs (often 0 for dynamic groups). Log the computed childWidgets.size() / sitemapWidgets.size() instead to match the actual traversal logic.

                                    logger.warn(
                                            "Widget id '{}' is invalid, index {} outside the number ({}) of widgets in the page",
                                            id, childWidgetID, ((LinkableWidget) w).getWidgets().size());
                                    w = null;
                                    break;
                                }
                            }
                        } else {
                            logger.warn(
                                    "Widget id '{}' is invalid, index {} outside the number ({}) of widgets in the sitemap",
                                    id, widgetID, sitemap.getWidgets().size());
                        }

[Copilot]

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java:902

• This default cached Text also stores the original parent, so the cache value can strongly retain the NestedSitemap key through parent.getWidgets(), defeating the WeakHashMap and retaining removed/updated sitemap graphs. This is especially likely for missing or item-derived sitemap names because defaultNestedSitemapWidgetsCache is never invalidated on sitemap registry changes.

                    Parent parent = nestedSitemap.getParent();
                    if (parent != null) {
                        text.setParent(parent);
                    }
                    // we return an empty nested text widget, so the link to the sitemap is visible in the UI
                    Text nestedText = Objects.requireNonNull((Text) sitemapFactory.createWidget(SitemapFactory.TEXT));
                    text.setWidgets(new ArrayList<Widget>(List.of(nestedText)));

[mherwege]

I need to keep the sitemapName DSL variable name to avoid conflict with the name variable on the sitemap itself. The syntax can be name=.

[mherwege]

I have a doubt if widgets id are properly handled in case of dynamic nested sitemaps.

I did quite some testing with switching sitemaps by changing the name, with multiple levels of sitemap references. So far it was OK, but I understand the doubt and the challenge.

[Copilot]

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

[mherwege]

Changing field names without proper testing afterwards is never a good idea. Fixed.

[mherwege]

Fixed

[mherwege]

Correct. Specific mock wasn't required, hence tests did pass.

[mherwege]

The rebuild workflow seems to be running into rate limits.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

[mherwege]

Done

[andrewfg]

@mherwege my own sitemap is a single file with 1200+ lines. See below where the > marks open nested pages. And in some cases the nested pages have their own > marks to open sub-nested pages. I am not sure if this PR would allow to split all those nestings and sub nestings into their own files .. but if it would, then that would be great!!

[mherwege]

this PR would allow to split all those nestings and sub nestings into their own files

That's the idea indeed.

[holgerfriedrich]

@mherwege is this something you want to have in 5.2?

[mherwege]

@mherwege is this something you want to have in 5.2?

That would be nice, but not critical. @lolodomo still wanted to have a look at it. I don't know if he has. The corresponding web PR is also in the pipe.