iTwin / appui

Monorepo for iTwin.js AppUi
MIT License
9 stars 2 forks source link

ToolbarGroupItem not accepting output of UiItemsProvider.getToolbarItems() #1112

Closed a-gagnon closed 1 week ago

a-gagnon commented 1 week ago

Describe the bug

Hey, I’m working on integrating our latest civil packages using the newer UiItemsProvider interfaces within pineapple.

What I'm trying to do is collect the output of multiple UiItemsProvider.getToolbarItems() and return that under a single group item (snippet below). The compiler is yelling at me because what getToolbarItems() return is ToolbarItem[] (which includes ToolbarCustomItem), and the ToolbarGroupItem I’m trying to create only accepts ToolbarActionItem | ToolbarGroupItem.

I was wondering if this is just an oversight, or it's by design? It seems to make composition a bit harder than it should be. I can use the isToolbarCustomItem and filter those out entirely, but I wanted to ask first. In this case, I'm the author of both the UiItemsProvider and the code that consumes it, so I know there is no custom item...

Snippet of code:

  public getToolbarItems(): ToolbarItem[] {
    let items: ToolbarItem[] = [];

    for (const provider of this.providers) {
      items = items.concat(provider.getToolbarItems());
    }
    return [ToolbarItemUtilities.createGroupItem({
      id: "my-complex-group-item",
      items,
      label: "Composition provider test",
      layouts: {
        standard: {
          orientation: ToolbarOrientation.Vertical,
          usage: ToolbarUsage.ContentManipulation,
        },
      },
    })];
  }

Thanks! Alex

To Reproduce

No response

Expected Behavior

No response

Screenshots

No response

Desktop (please complete the applicable information)

No response

Additional context

No response

GerardasB commented 1 week ago

ToolbarGroupItem API was intentionally designed to disallow custom items (this was related to challenges in old/outdated designs) - that's why group items can only contain other group or action items. I think we could revisit this with our new toolbar implementation, but that would fall under new feature request instead. Your approach of filtering the custom items out seems correct to me.