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

Make diagram resize configurable #33

Open
NiklasRentzCAU opened this issue Apr 3, 2019 · 4 comments
Open

Make diagram resize configurable #33

NiklasRentzCAU opened this issue Apr 3, 2019 · 4 comments

Comments

@NiklasRentzCAU
Copy link
Contributor

I am talking specifically about this line:
https://github.com/eclipse/sprotty-theia/blob/e14752e73bd553881891355a44d3dc6173ea9228/src/theia/diagram-widget.ts#L142

Currently on resize the zoom level is always reset with the CenterAction that is forced to be executed here. I would like to have this configurable so that a FitToScreenAction or no action at all is executed instead of the CenterAction.

@JanKoehnlein
Copy link
Contributor

Why not just override DiagramWidget#onResize in your own subclass?

@NiklasRentzCAU
Copy link
Contributor Author

I also had that option in mind, but I think the configurability on the Sprotty side leads to better maintainability on projects that want different behavior, such as our project.

Overriding this method would require to also reimplement the other functionality in the method - reading the bounds from the internal node property, initializing the canvas with that and calling the super method. While overriding the method, the super.super implementation can obviously not be called, so the concept of inheritance gets broken in this method, if I just do not want to call the center action, but keep all other functionality the same.

One thing I just noticed is that the super call actually does not do anything, so yes, just overriding is possible without breaking the code, but I think my point is still valid. Why does the onResize function even call the super implementation, if it is a no-op?

@JanKoehnlein
Copy link
Contributor

PRs are welcome

@spoenemann
Copy link
Contributor

I removed the center action in #63. I'll put it into a separate method to make it better configurable.

@spoenemann spoenemann self-assigned this Jul 31, 2020
@spoenemann spoenemann removed their assignment Apr 26, 2023
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

No branches or pull requests

3 participants