-
-
Notifications
You must be signed in to change notification settings - Fork 227
Fix inspector depends functionality. #710
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
Conversation
- get property value from root surface object
- pass objectlist property name to surface object
|
@mjauvin could you add a test scenario to the Test plugin that demonstrates this? |
|
@bennothommo I'll try, but I don't garanty anything |
|
@bennothommo I'm frankly not sure how to approach this. The tests would need to create a inspector widget then simulate trigger events on the dropdown fields... any idea? I could propably instanciate such a widget with |
|
@mjauvin I assume you're using this functionality in a component in a plugin? We don't need an automated test case for this at the moment, just a reproduction of what the use case is that triggers the original issue so we can test the before / after of the fix. Can be implemented as a component within the test plugin I'd imagine. |
|
yeah, I think this is a better approach. |
|
@LukeTowers @bennothommo this commit (wintercms/wn-test-plugin@5e93893) adds a component to the Test plugin that can be added to a page/layout to demonstrate the problem.
This PR fixes both of the above issues. |
|
I'll have to give this a test in a couple of days - launching that major project I've been working on in the next day or two, so gotta focus on that going smooth :) |
Did you get time to test this? |
|
I haven't, no. Thanks for the reminder. |
|
Is that going to be superseded by the inspector widget rewrite ? |
|
Most likely but the Inspector rewrite is a ways off still - if this works, I'm happy to merge it. It's all tested, I assume? EDIT: I realise I was supposed to test it - d'oh! I'll get to that today. |
|
@bennothommo Yes, it's been tested on my end and there's a procedure/PR to test this with the test plugin above. |
This PR corrects the following problems:
objectinspector type: properties usingdependsdo not refresh.objectListinspector type:itemPropertiesusingdependsdo not work.