-
Notifications
You must be signed in to change notification settings - Fork 110
Update to include function to open an alert panel relative to a window #371
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
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 would be great to fix the issue I found, but then somebody already broke the code and your change isn't any worse that that.
|
It feels very wrong when you tell me that a PR is complete and I should review it and after the review you make significant changes that are unrelated to my comments. I should start again with a complete review but then you probably will change something else. You do understand this is the main reason I normally wait at least a month before reviewing your code? |
I actually meant to mark this one as a draft. As it has a couple of issues. It won't happen again. You don't have to delay a month or so. I'll do better. |
|
I will be more careful in the future. You won't have to delay your reviews. I should have marked this one as a draft so that you wouldn't review it too soon. The issue is that the panel is not coming to the front as it should and I am not sure why. I will mark this as ready when I have addressed this problem. |
|
I may have a few very minor fixes after this. I will address the concern you have above. |
|
I believe this PR is done @fredkiefer. No further changes. |
|
I have addressed the comment and I reverted the code to close to what was originally approved with the fix I mentioned to the order of parameters. I am going to proceed with the merge. |

Add new function to bring up a panel relative to a window