[pdm-devel] [PATCH proxmox-datacenter-manager v2 14/15] ui: sdn: add evpn overview panel

[Stefan Hanreich]

On 9/2/25 11:30 AM, Dominik Csapak wrote:
> On 9/2/25 8:34 AM, Dominik Csapak wrote:
>>
>>
>> On 9/1/25 5:02 PM, Stefan Hanreich wrote:
>>> On 9/1/25 3:44 PM, Dominik Csapak wrote:
>>>> one question: couldn't have the toolbar stayed the same as before?
>>>>
>>>> as in, create the toolbar in line in the `main_view` and just `clone()`
>>>> it once for each panel?
>>>>
>>>> not that i'm totally against factoring such things out, but the
>>>> `EvpnToolbar` does not do anything special FWICT, so creating
>>>> the toolbar inline would have been fine...
>>>
>>> I had that initially, but cloning the toolbar had issues with the
>>> positioning of the dropdown (it was stuck to the top left of the
>>> screen). Not cloning the toolbar but creating it twice fixed it for me.
>>>
>>
>> this sounds like a bug we should fix ;)
>>
>> i'll see how that happens and see if i can send a fix for that.
>>
>> for the time being we can ofc leave it as is and clean up when we have
>> a fix
>>
>>
> 
> for the record for the list:
> 
> the issue here was that every component in our toolkit has a
> 'NodeRef' (in their std_props) that is linked to a Dom element on rendering
> 
> In the MenuButton we use this NodeRef from the std_props as the anchor
> for aligning the menu.
> 
> When we clone some component's properties, we clone the NodeRef too
> but that messes up the references to the dom, since now the clone
> points to the same noderef
> 
> The first rendered menu works fine, but when the second one is rendered
> the link from noderef <-> dom changes to the second one. Now when one is
> trying to use the menu on the first tab, the noderef points to an
> element which is invisible, which fails to align properly.
> 
> We're still deciding how to cope with cloning properties and their
> node_refs (we could make it optional, make sure aligning always
> require internal node_refs, impl clone such that the nodref
> is not copied, etc)
> 
> the easiest fix here is to simply have a
> 
> fn create_toolbar(...) -> Toolbar
> 
> function that returns a new toolbar each time
> and use that twice. This is still much shorter
> than having a full component for basically the `view` method.
> 
> Can be cleaned up as a follow-up IMO

I'll send a v3 (together with the other nit), shouldn't be too hard to
change imo.