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

Saving and position columns bug fixes and changes #6067

Merged
merged 4 commits into from
Nov 11, 2020

Conversation

dannyparsons
Copy link
Contributor

  1. Fixed functionality so that when replacing an existing column, it stays in same position by default and also added parameter to the R methods to allow it to be moved when replaced (a control for this can be added to the positioning sub dialog in a separate PR)
  2. Fixed bug in calculator where the before parameter, and therefore positioning a new column, was changed when reopening the dialog
  3. Added strAdjacentColumn as a parameter to assign to methods as this is now part of the saving options

@africanmathsinitiative/developers This is ready for the review. 1. needs to be reviewed across dialogs that use the save control, please test carefully as this edits core parts of saving columns, and 2. only needs to be checked for the calculator dialog

dannyparsons and others added 3 commits October 22, 2020 17:51
… added parameter to R code to allow existing columns to be moved when replaced, fixed bug in calculator where before parameter was reset on reopening, added strAdjacentColumn as parameter to assign to methods
@lloyddewit
Copy link
Contributor

@Patowhiz You've done quite a bit of work in this area, please could you peer review? thanks

rdstern
rdstern previously approved these changes Nov 10, 2020
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.

Good

Copy link
Contributor

@Patowhiz Patowhiz left a comment

Choose a reason for hiding this comment

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

This looks good. Tested it across multiple dialogs that use the ucrSave control. ucrSave control will need to be modified to enable positioning of existing columns and take advantage of the new parameter added. Issue #6002 will also need to be reopened to reimplement the functionality requested.

Just a few questions to @dannyparsons.

instat/static/InstatObject/R/instat_object_R6.R Outdated Show resolved Hide resolved
instat/clsRCodeStructure.vb Show resolved Hide resolved
@Patowhiz
Copy link
Contributor

@dannyparsons @lloyddewit, on a separate note, now that @dannyparsons has worked on the SetAssignTo() subroutine, I think in line 218 If bUseBaseFunction Then 'TODO SJL 07/04/20 Use ElseIf? of clsRSyntax.vb.
The todo could now be done by adding the ElseIf as recommended

@dannyparsons
Copy link
Contributor Author

Thanks, done and addressed the comments.

@Patowhiz
Copy link
Contributor

@lloyddewit this looks good to me.

@dannyparsons dannyparsons merged commit a97a824 into IDEMSInternational:master Nov 11, 2020
@dannyparsons dannyparsons deleted the save-position branch November 11, 2020 16:59
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.

4 participants