Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reopen new diagram widget on start for current active editor. #100

Merged
merged 4 commits into from
Jul 25, 2022

Conversation

soerendomroes
Copy link
Member

Since the old webviewPanel does not refer to the old SprottyWebview (which is a KlighdWebview) cannot be readded to the list of available KlighdWebviews in KlighdExtension and the old panel cannot be used during the creation of a new KlighdWebview since it is readonly, we recreate the Diagram on the start of the extension for the currently active editor.

Copy link
Member

@NiklasRentzCAU NiklasRentzCAU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is very nice that it is possible to reopen the diagram on startup, albeit not using the VS Code API for that. For the SCCharts extension I can see this being the wanted behavior, as the diagrams always belong to the extension. As this is implemented here in klighd-vscode however, I would like this feature to have some slightly different behavior to avoid users that have this and an extension with very generic syntheses installed to always open (not reopen) a diagram on startup, even if they manually closed it in the previous session.

I would suggest to only restore the view when it was opened before (either via VS Code API or maybe via some persistent / session storage). Furthermore, what about this being an option to open a diagram automatically on startup or not?

I assume that the second editor group is the one of the diagram, which allows it to be placed below or beside.
Copy link
Member

@NiklasRentzCAU NiklasRentzCAU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one debug statement left, please remove that. Otherwise, 👍

applications/klighd-vscode/src/extension.ts Outdated Show resolved Hide resolved
Copy link
Member

@NiklasRentzCAU NiklasRentzCAU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@NiklasRentzCAU NiklasRentzCAU merged commit 818c832 into main Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants