This was originally meant as a demo of how to convert a tree data structure into the shape expected by EuiContextMenu, but it's been copy-pasted from the docs and used in several places in X-Pack. We can dedupe Kibana code and simplify maintenance a bit by making this part of EUI's interface and adding a couple tests. Low-hanging fruit.
I'd like to take this up @cjcenizal
Sounds great @jshreyans ! Let us know if you need any help.
Might make sense to provide this utility method as a property on the EuiContextMenu, as it's use seems pretty specific? Or, if it is moved to a top-level utility/service export, I'd look into making the parent/child relationship keys configuration, e.g. the line
item.panel = item.panel.id;
could be configurable with additional method parameters and transform into
item[panelKey] = item.panel[panelKey];
@cjcenizal Sorry I couldn't get a chance to work on this earlier. Could you explain what exactly needs to be done here. I'm new to the codebase and all these terms are still quite new for me.
@cjcenizal can I take this issue up if no one is engaged with it?
@tathagat2006 Thanks for offering but I think @jshreyans got to it first! I believe there are still a number of other open issues with the “good first issue” label that need owners though.
@jshreyans I will defer to @chandlerprall in terms of next steps. I’m on leave this week and as an active maintainer of the repo he is also better positioned to advise you.
@jshreyans the original ask came from identifying flattenPanelTrees usefulness in EUI's docs/examples and thinking it is probably useful for consuming applications. In fact, we've copy&pasted the function so it exists in 4 of our docs' files:
https://github.com/elastic/eui/blob/75fee43023fbe7ed32cf437cd711322d4228a7e0/src-docs/src/views/context_menu/context_menu_with_content.js#L14
https://github.com/elastic/eui/blob/f5d0aa5ee1b259ddcd86ecd4869a0e92dd758606/src-docs/src/views/suggest/global_filter_item.js#L12
https://github.com/elastic/eui/blob/f5d0aa5ee1b259ddcd86ecd4869a0e92dd758606/src-docs/src/views/suggest/global_filter_options.js#L10
https://github.com/elastic/eui/blob/6d3829a2c580dcee3ec5a70c1f13e76e4d87566c/src-docs/src/views/context_menu/context_menu.js#L13
The resolution to this issue is to extract that flattenPanelTree method into a single utility service somewhere in https://github.com/elastic/eui/tree/93c34b28f752fac3df01f84331601c57f072cf1c/src/services - glancing through that directory, it probably should be its own top-level file at _src/services/flatten_tree_panel.ts_
At minimum, these steps would need to happen:
flattenTreePanel into EUI's servicesflattenTreePanel from _src/services/index.ts_Additionally, it's worth looking into making the flattenTreePanel method expandable with another parameters, which could be used to identify the name of the object key to use when flattening, e.g.
function flattenPanelTree(tree, keyName = 'panel', array = []) {
array.push(tree);
if (tree.items) {
tree.items.forEach(item => {
if (item.panel) {
flattenPanelTree(item.panel, keyName, array);
item[keyName]= item[keyName].id;
}
});
}
return array;
}
in addition to the keyName, perhaps the name of the id property should be configurable as well
@chandlerprall thanks a lot for the help! Will send a PR soon
@chandlerprall When I write the flattenPanelTree function into its own file in the services directory, the type checker prompts me to specify the types for tree and item variables. What would be the respective types for both?
For now, I've left both to type any
This is a pretty terrible function, ha. I've removed the configurable keyName that I originally suggested as it doesn't play well with making this function more flexible, and I don't think it's worth the effort. This is the typing I came up with:
type ItemIn = Omit<EuiContextMenuPanelItemDescriptor, 'panel'> & {
panel?: PanelIn;
};
type PanelIn = Omit<EuiContextMenuPanelDescriptor, 'items'> & {
items?: ItemIn[];
};
export function flattenPanelTree(
tree: PanelIn,
array: EuiContextMenuPanelDescriptor[] = []
) {
array.push(tree);
if (tree.items) {
tree.items.forEach(item => {
if (item.panel) {
flattenPanelTree(item.panel, array);
(item as EuiContextMenuPanelItemDescriptor).panel = item.panel.id;
}
});
}
return array;
}
Right, this does it. Also, are we looking to add any tests for this function?
Yes, having a test or two would be great.
👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.
I'm going to leave this one open as we further reiterate and improve on our docs.
Most helpful comment
Sounds great @jshreyans ! Let us know if you need any help.