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

Ensured position works correctly in Calculator dialog #8470

Merged
merged 4 commits into from
Aug 6, 2023

Conversation

N-thony
Copy link
Collaborator

@N-thony N-thony commented Aug 2, 2023

Fixes #8434
@rdstern have a look. Thanks.

@rdstern
Copy link
Collaborator

rdstern commented Aug 2, 2023

@N-thony I am confused on which issue it fixes, given the title?

@N-thony
Copy link
Collaborator Author

N-thony commented Aug 2, 2023

@N-thony I am confused on which issue it fixes, given the title?

@rdstern sorry, issue #8434

rdstern
rdstern previously approved these changes Aug 2, 2023
Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@N-thony I like the change, so I am approving.
a) I still think there could be a bug in the position control, but I would be happy to test that separately later unless you have found something obviously easy to fix?
b) It doesn't seem to work in quite the same way for the data frames. In hindsight, I am actually happy that it is different - so you can choose to overwrite a column, but not a data frame. (If you want to get rid of a data frame then you have to delete it!
c) The other change that affects all dialogs, that I am very keen on, is that when you return to a data frame the focus is always on the first receiver (so not on the receiver that you left it on.) Patrick said that is also just a change in a single line. Can you add that here - or in a second pull request - or should Patrick do it?

rdstern
rdstern previously approved these changes Aug 4, 2023
Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

Great - seems fixed

Copy link
Contributor

@lloyddewit lloyddewit left a comment

Choose a reason for hiding this comment

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

looks good, just one small question

instat/ucrInputComboBox.vb Outdated Show resolved Hide resolved
@lloyddewit lloyddewit added the bug label Aug 6, 2023
@lloyddewit lloyddewit changed the title Fixed variable position bug on Calculator dialogue Ensured position works correctly in Calculator dialog Aug 6, 2023
@lloyddewit lloyddewit merged commit 74e80ab into IDEMSInternational:master Aug 6, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Position sub-dialog may not always work correctly and overwriting an existing variable is too easy!
3 participants