Please check change to controls bar

[marcolarosa]

HI @beepsoft, @csontosreka

Can you please check the revision to the controls bar in this branch: https://github.com/describo/crate-builder-component/tree/rework-controls-bar

I've changed the positioning, type and styling of the reverse links control and I think this affects you guys. The original layout was causing me issues with alignment and I've never liked the different styling - I shouldn't have done that.

Original:

Revision:

I'd like to release a point release of the component if possible.

Thanks!

[beepsoft]

Yes, this now looks a bit strange for us as we don't use any of the builtin buttons and moved this functionality to our own buttons above describo with added controls (undo-redo):

A solution could be if we had a way to control the visibility of the reverse link panel from the outside of the component, eg. via a showReverseLinkBrowser and this could show the panel, while enableReverseLinkBrowser would control whether the button is displayed or not.

[marcolarosa]

Yeah - i thought there might be a problem.

I've just pushed an update where you can control it from outside. Set enableReverseLinkBrowser and showControls to false. Then, in your app get a handle to the internal component.

https://github.com/describo/crate-builder-component/assets/2639995/9230bb16-1194-49e3-acde-e5d738c01b2f

The code is at:

https://github.com/describo/crate-builder-component/blob/rework-controls-bar/src/app/EmbeddedComponent.component.vue#L245-L248

We can expose more internal functionality that way too if it helps. Not sure if you notice but the entity browser now enables filtering by type.

The docs for this functionality are at https://describo.github.io/docs/component/advanced-usage.html

[beepsoft]

Thanks Marco!

The problem is that I cannot call a function from React to Vue* I can only:

• pass properties
• define callback functions, that Vue can call

So instead of an imperative toggleReverseLinkBrowser() it would be better to be able to add a property to declaratively set the visibility state of the reverse link browser. And then I could do the toggle logic in my React app and pass true or false as this property's value.

Would this be possible?

* I actually can, but that would be called at random (number of) times when the component rerenders and that would cause an unknown number of calls to toggleReverseLinkBrowser().

[marcolarosa]

Ahh no worries!

I've just pushed an update with a new option showReverseLinksBrowser. You can try it in the app. The only issue with this (as you'll see in the app) is that the toggle doesn't go back to false when the reverse links browser closes. I'm not sure how to handle that other than emitting an event which would be messy (ie having a special event emitted just for that one UI interaction).

Let me know how you go. I think I'm done for tonight so happy for you play around with some options if you have time.

Thanks!

[beepsoft]

showReverseLinksBrowser works great, thank you! Yeah, we probably need some callback from describo when the user closes the reverse link browser, so that the outside component (react) can sync visibility state, ie. set showReverseLinksBrowser to false for the next render cycle.

[marcolarosa]

This has strayed into territory I'm not super comfortable with.

The whole point of the component is that it is a self contained drop in RO crate editor. I have no problem with you guys having the controls in your layer but mixing controls between your layer and the component as we are doing here doesn't really make sense to me. As we've both seen, to support that the component would now need to emit an event about part of the internal state. So If we continue down this path then we have two properties (showReverseLinksBrowser and enableReverseLinkBrowser) and an event (yet to be emitted) for one capability.

And it actually overlaps with the capability exposed by the component itself (which I implemented as a clean way to change the internal state from outside).

I think maybe the reverse link browser needs to be something implemented by you guys and we simplify the config in the component so that the control for it is part of the standard controls. Then, you guys can just turn off the whole control bar (including the reverse link control) as you currently do as you are implementing the features you want in your layer.

I'm stuck by this because the original controls layout were causing layout issues in my app so I need to push this forward to get this new layout noting that this is not resolved for you.

I'm going to merge these changes to move forward but I'm leaving this ticket open to keep discussing a path that works for you guys too. I'm not sure what that is yet but let's keep thinking about it if that's ok with you. Hold off on updating the react component until we have a solution that's acceptable to you too.

[marcolarosa]

Just for context, here's why this change was made.

The original controls layout. Notice the misalignment across the top and the extra whitespace down the right side because of the reverse links control.

The revised layout:

[beepsoft]

And what about an option specifying the placement of the browser trigger button? Either above (like now) or right to the component (the "old" way). How do you feel about that?

[marcolarosa]

That could work. And it would only need the original config property.

I could integrate the new control into the controls component so setting that to false turns it all off. And the old control could be toggled on off with the original option. I'll have a tinker with it tomorrow and get back to you. Good idea @beepsoft (assuming I can implement it ;-)

[marcolarosa]

https://github.com/describo/crate-builder-component/releases/tag/v0.74.0

It worked and was surprisingly easy. Kicking myself for not thinking of it....

Anyway, when you update the react component, can you please set enableReverseLinksBrowser to false by default so it's consistent with the config in the vue component. And in your use case use showControls: false and enableReverseLinksBrowser: true

[beepsoft]

Is it possible that setting showControls doesn't work now?

screencast-localhost_9000-2024.06.07-09_31_37.webm

[marcolarosa]

Umm yeah... sorry! Just pushed a bugfix and built 0.75.2 with it

[beepsoft]

Thanks, it works all right now!