Please review : https://github.com/describo/crate-builder-component/tree/revise-routing-config

[marcolarosa]

Hi @beepsoft - can you please review this and let me know what you think?

This branch contains a very big change to the components internal state management.

Currently, the component has a switch enableInternalRouting that turns on / off whether the component updates the browser URL with the entity id. I've been thinking about this a lot and it doesn't make any sense for the component to do this as the component is meant to be embedded inside something else that probably has its own routing. That's why we have this toggle - so we can turn it off and not affect the routing being done by the outer app. I know you do this and I do as well.

But, we do have the controls in the nav bar to go back and forth so really, the component should have an internal state that is wired up to those controls but which doesn't touch the browser url (and browser history). That way, users like you can just turn off the whole control bar and do it themselves whilst users like me can have it work, without it clashing with the url management being done by the outer app.

So, with this change there is now an internal editor state. It's still pretty basic (it just tracks id changes for now) but I expect I can use this to remember things like which tab an entity was on and other bits and pieces related to a view at any given time. It's also available to the outside via defineExpose (like cm is) so that the outer app can navigate the state back and forward. Everything else regarding setting the entity id from outside is the same.

To get this to work inside and out I needed to install pinia which is a vue state management lib. The outer app doesn't need to set it up - the component sets it up on load. The provide / inject pattern just wasn't working with updates across the app though I don't know why. But with pinia it works nicely. Because of this there are a lot of code changes because i'm sharing the editor state and the configuration via pinia (configuration was provide / inject). I might migrate all of the provide/injects across the app to pinia but not decided yet.

At the moment I have lots of debugging enabled in the component / app to test this. You'll see across the top the history inside and outside the app as well as controls to go back and forward in the control bar in the component and those in the outer app. There's logs in the console to watch as well.

https://github.com/describo/crate-builder-component/assets/2639995/6f0eb8c3-e402-45fa-8da3-3a9a2f610a3f

For your case, even though it uses pinia and internal state, nothing should change. Turn off the controls in the component and keep doing what you do and it all should work as it does now.

[marcolarosa]

@beepsoft

This commit (https://github.com/describo/crate-builder-component/commit/68cbf0299fc789afb09da6934cb48b47586b5e48) in this branch now stores the tab in the internal state. It works with the controls in the dev app but I'm not sure how it will interact with your controls outside.

If it works as expected, then it also removes the need for the props: resetTabOnEntityChange and resetTabOnProfileChange. The key change in this regard is at https://github.com/describo/crate-builder-component/blob/68cbf0299fc789afb09da6934cb48b47586b5e48/src/crate-builder/RenderEntity/Shell.component.vue#L340-L400 (I've just commented out the old code for now).

[beepsoft]

Thanks @marcolarosa, I will take a look soon!

[beepsoft]

In general it seems OK, but I have to figure out how I can control the back/forward from react. In vue you keep a ref of describo and call functions directly on it. I will try to somehow expose this describo ref to react as well so that react component users can call these functions directly.

[marcolarosa]

Whilst it would be good if you were able to, I don't follow why it's necessary?

In normal operation, when showControls is true, it all happens internally with the UI buttons in the component. And if a user sets showControls to false, then it doesn't matter what's happening inside as presumably they are keeping their own state and navigating back / forward by setting the entityId property. Isn't that how you guys use it?

I'm particularly interested that the commit I ref'ed above doesn't conflict with changing state from outside by setting entityId. If it does, I think i'll need to add a prop like enableEditorState true | false in order to turn that behaviour off so that it works like your original code did. I hope that makes sense.

[beepsoft]

Yes, we can navigate directly to an entity by setting the entityId - although it doesn't keep the tab selection state in this case. Would describo handle it internally?

I've just seen in the develop app that you have these external back/forward buttons and I thought it would be nice if the same could be done from the embedding react app as well.

[marcolarosa]

Yes, we can navigate directly to an entity by setting the entityId - although it doesn't keep the tab selection state in this case. Would describo handle it internally?

It doesn't at this stage and I can't really imagine how it could.

I've just seen in the develop app that you have these external back/forward buttons and I thought it would be nice if the same could be done from the embedding react app as well.

That's the better way I think. And if you can wire this ref up via the react app then react users can get at the internal crate manager to manage updates from outside which means whole reloads aren't required because the crate was changed via the prop.

[beepsoft]

Yes, we can navigate directly to an entity by setting the entityId - although it doesn't keep the tab selection state in this case. Would describo handle it internally?

It doesn't at this stage and I can't really imagine how it could.

If we could set eg. a tabName together with entityId, that would solve it. Your internal navigation already keeps track of these two and resets them when navigating back/forward so the basic mechanism for this is already there, I guess.

[marcolarosa]

Yes, we can navigate directly to an entity by setting the entityId - although it doesn't keep the tab selection state in this case. Would describo handle it internally?

It doesn't at this stage and I can't really imagine how it could.

If we could set eg. a tabName together with entityId, that would solve it. Your internal navigation already keeps track of these two and resets them when navigating back/forward so the basic mechanism for this is already there, I guess.

I would prefer it if we could avoid adding more properties as that means watchers and keeping props in sync with the internal state and we still don't know if it's back or forward anyway. If you can wire up the ref it's a neater solution that allows me to expose even more of the internal state to the outside without extra props, and watchers, and keeping things in sync for anyone to control as they wish. Would it be ok to investigate?

[beepsoft]

I definitely try to investigate passing the describo ref to react, but it will take same time as I will go to vacation.

[marcolarosa]

Yeah - that sounds like much more fun.

Are you ok with me building a release for now and leaving this ticket open until we get through it?

[beepsoft]

Sure, go ahead!

[beepsoft]

So far it seems that passing the describo internals exposed from Shell.component.vue to the React world works quite straightforward!

This opens up a lot of new possibilities for integrations! 🎉

[marcolarosa]

That's awesome - thanks @beepsoft !

[marcolarosa]

ok - i've just merged the branch in master. I'm going to remove this branch from github as I don't think you need it. Just be aware that it's gone and you can work in the react component from master.