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

GLSP-1388 Enhance extensibility of ContainerManager #385

Merged
merged 2 commits into from
Aug 21, 2024
Merged

Conversation

ndoschek
Copy link
Contributor

@ndoschek ndoschek commented Aug 14, 2024

What it does

  • Added IContainerManager interface for easier customization
  • Consume IContainerManager for dependency injection
  • Extract isCreationAllowed submethod for element insertion checks

Resolves eclipse-glsp/glsp#1388

How to test

Node creation should work as before, this should not impact the functionality at all.

Follow-ups

--

Changelog

  • This PR should be mentioned in the changelog
  • This PR introduces a breaking change (if yes, provide more details below for the changelog and the migration guide)

@ndoschek ndoschek requested review from tortmayr and martin-fleck-at and removed request for tortmayr August 19, 2024 12:40
- Added IContainerManager interface for easier customization
- Consume IContainerManager for dependency injection
- Extract isCreationAllowed submethod for element insertion checks

Resolves eclipse-glsp/glsp#1388
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

Hi Nina, thank you for the PR. It has opened a few questions for me, so hopefully we can get that sorted quickly. Otherwise, everything works as expected.

- Ensure backward compatibility for ContainerManager binding by using bindAsService.
- Make isCreationAllowed public and reuse in KeyboardPointerPosition (accessibility tools)
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

Thank you for the quick turnaround! LGTM!

@ndoschek
Copy link
Contributor Author

Thank you for the review @martin-fleck-at!
The Eclipse CI was down today, but the Jenkins build was now completed finally.

@ndoschek ndoschek merged commit 4d96e60 into master Aug 21, 2024
7 checks passed
@ndoschek ndoschek deleted the issues/1388 branch August 21, 2024 14:37
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.

Enhance extensibility of ContainerManager
3 participants