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

Show edit window and buttons below changes when using PageForms for chameleon version 4.4.x #436

Open
wants to merge 3 commits into
base: 4.x
Choose a base branch
from

Conversation

WouterRademaker
Copy link
Contributor

Should solve #319.
@ckapop found the issue that was causing this error and a solution. The changes were made suitable for and tested on Chameleon 4.4.1 by @WouterRademaker.

Same changes as #406

@WouterRademaker WouterRademaker changed the title Show edit window and buttons below changes when using PageForms for chameleon version 4.x Show edit window and buttons below changes when using PageForms for chameleon version 4.4.x Aug 5, 2024
@malberts
Copy link
Contributor

malberts commented Oct 9, 2024

@WouterRademaker the failing tests suggest the new getHtml() is not getting mocked correctly. So this is a problem with this PR, not due to unrelated CI failures.

@WouterRademaker
Copy link
Contributor Author

WouterRademaker commented Oct 19, 2024

@malberts, but does this PR provide inspiration on how to solve this issue?

@WouterRademaker
Copy link
Contributor Author

@malberts, how do we get the new getHtml() mocked correctly? (If that is the right term)

@malberts
Copy link
Contributor

malberts commented Feb 2, 2025

@WouterRademaker I had a quick look at the code and this does not seem like the right approach.

The Component base class is supposed to get a ChameleonTemplate injected via constructor, and also has a getSkinTemplate() method for accessing it. However, in this PR there are no changes to how it is accessed, or even any other logic changes. It only replaces the existing instance of that ChameleonTemplate. If that fixes the problem, then it suggests there is something wrong with how that initial instance is created. The solution would require fixing that, not passing around a new instance and then replacing the existing instance after the component was already created.

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