Add reveal in editor button to method modeling panel

[koesie10]

This adds a new reveal in editor button to the method modeling panel:

Once clicked, it will:

• Expand the group (library/package) in which the method is located
• Scroll to the method
• Highlight the method

Screen.Recording.2023-10-02.at.15.35.06.mov

It's easiest to review this commit-by-commit, I've tried to create somewhat separate pieces.

Checklist

• CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
• Issues have been created for any UI or other user-facing changes made by this pull request.
• [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

[charisk]

Thanks for getting this working Koen! I also appreciate the separate commits.

A question on the data flow: I was wondering why you chose to introduce a model editor view tracker versus sending messages to the store. Would the following work?

• The modeling store holds some state around the highlighted method
• The method modeling panel sets this state
• The model editor reacts to changes to this state

I'm thinking about this because most other flows are through the data store now (not all of them yet though) so it seems it would be simpler to follow a similar pattern if possible and avoid having to track model editor views. Appreciate that this kind of state is more ephemeral, but I think that's still okay to be managed by the modeling store.

Feel free to tell me I'm completely wrong 😬

[koesie10]

A question on the data flow: I was wondering why you chose to introduce a model editor view tracker versus sending messages to the store. Would the following work?

I did think about that option, and I don't think we would even need to store this state. We could just have an event emitter for "reveal method". However, I didn't really think it was part of the modeling store, so wanted to keep it separate. Since there's a separate issue for only having 1 model editor per database, we would probably need something like this view tracker anyway to ensure that only 1 instance of a model editor is present for every database. I'm happy to change it to something else though, I just didn't want to put everything in the modeling store, even if it's unrelated to the actual modeling.

[charisk]

I did think about that option, and I don't think we would even need to store this state. We could just have an event emitter for "reveal method"

That's true, we could extract modeling events/event emitters out of the store. That's something that crossed my mind before but it didn't feel like it was worth it at the time, but perhaps it makes sense to separate modeling events and store.

Since there's a separate issue for only having 1 model editor per database, we would probably need something like this view tracker anyway to ensure that only 1 instance of a model editor is present for every database

For this I was thinking that we already have state around databases being modeled (in the modeling store), so we could use that information to enforce that only 1 instance of the model editor is present for each db. I was hoping we wouldn't have to store more state (tracking views), but I can't say I've thought this through enough to be confident in this.

We could go with what you've implemented too, my only reservation is that it introduces one more thing to track so it feels like the complexity is a bit higher. Any thoughts/preferences?

[koesie10]

That's true, we could extract modeling events/event emitters out of the store. That's something that crossed my mind before but it didn't feel like it was worth it at the time, but perhaps it makes sense to separate modeling events and store.

I think having the modeling events in the store is fine since those give notifications about changes to the data, but I don't think "reveal this method" is something that changes the data, so that's why I'm hesistant to put it in the store.

For this I was thinking that we already have state around databases being modeled (in the modeling store), so we could use that information to enforce that only 1 instance of the model editor is present for each db. I was hoping we wouldn't have to store more state (tracking views), but I can't say I've thought this through enough to be confident in this.

I think "the existing model editor view should be focused" is hard to implement in that way, unless we're also exposing an event like "focus this model editor view". I think that would also work, but then we'd probably need some kind of manager/store for those events which are unrelated to the modeling data. I can't really think of a good name for it, do you have any suggestions?

We could go with what you've implemented too, my only reservation is that it introduces one more thing to track so it feels like the complexity is a bit higher. Any thoughts/preferences?

That is true, but we're already doing this for the variant analysis views as well, so I don't think it introduces a new concept into the codebase. I'm happy to do it either way, but I think we should separate these "view events" from the "modeling data events", and I don't have a good idea of what to call that.

[charisk]

Okay cool, let's go with this and I'll try some ideas I have out in code and can chat about it later on. Thank you for a productive discussion!