-
Notifications
You must be signed in to change notification settings - Fork 8
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
Update ParameterIdentificationFeedbackViewVisibilityUICommand.cs #1486
Update ParameterIdentificationFeedbackViewVisibilityUICommand.cs #1486
Conversation
yes much better. This would have created. a leak the way it was before because the PI was not cleared anyways |
|
||
namespace OSPSuite.Presentation.UICommands | ||
{ | ||
public class ParameterIdentificationFeedbackViewVisibilityUICommand : ObjectUICommand<IParameterIdentificationFeedbackPresenter>, IListener<ParameterIdentificationSelectedEvent> | ||
public class ParameterIdentificationFeedbackViewVisibilityUICommand : ObjectUICommand<IParameterIdentificationFeedbackPresenter> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is in fact a better UI command for this. ActveSubjectUIComand. Can you use this insteaD?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ActveSubjectUIComand
I did not find ActveSubjectUIComand but instead I found ActiveObjectUICommand. The problem is that we need the command to implement ObjectUICommand but retrieve a ParameterIdentification as the active subject. So if I would extend ActiveObjectUICommand the Subject will be of type IParameterIdentificationFeedbackPresenter which is not what I need. To avoid this, I would need to change IParameterIdentificationFeedbackManager so it uses the cache based on the presenter, and not the object. This will imply to make the presenter to have a public getter for the object so we can access the object from the Subject (presenter). Do you like these changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand. Why can't you be ActiveObjectUICommand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was about making the ParameterIdentification public on the presenter interface (IParameterIdentificationFeedbackPresenter). Sorry the description was not that clear. Check changes on the code for easier understanding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sorry but I don't understand how this works. Can we have a call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sorry but I don't understand how this works. Can we have a call?
@msevestre Just call any time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good
Fixes #1372 (Using ActiveSubjectRetriever to retrieve the active parameter identification instead of listening to events)