Improved the column right-click menu and the Edit Cell dialog

[derekagorhom]

fixes #8366
fixes #8239
This fixes the issues of some of the properties missed when creating the Edit dialog

@N-thony this PR is ready for review

[N-thony]

fixes #8366 This fixes the issues of some of the properties missed when creating the Edit dialog

@N-thony this PR is ready for review

can you reduce the space as shown in red mark and also in the Property, StartPosition = CenterScreen

[derekagorhom]

@N-thony I have made the changes, can you have a look
Thanks

[rdstern]

@derekagorhom and @N-thony seems as good as before to merge, so I am approving. In the additions later, I note that you can (in the dialog) change the variable or the row, but neither work. So they should be disabled. They are for information and viewing in this dialog and not for editing.

[derekagorhom]

@rdstern I was able to make the row ReadOnly since it has that property but the column it does not have this property.
@N-thony is it possible to make the column ReadOnly ?

[rdstern]

@derekagorhom I note that the focus (in turquise) is on the column. Could you not have that? You can't change an item in a receiver unless it has the focus.

[rdstern]

@derekagorhom I like the spacing and particularly that the column and row remain visible, but are disabled. Great.
But the Factor columns have taken a backward step. They have lost the drop-down that gives the choice between the different levels?

[derekagorhom]

@rdstern i brought back the old selector because the new one was the reason the factor dropdown was not working and i also had a discussion with @N-thony and i realised that The receiver will always be highlighted since it is linked to the selector because that is how it has been done internally, we are still looking for ways to solve this.
In the meanwhile i have already added the date control for the dialog and it work well now.
Can this PR be kept open while i work on the logical option so that this can also close #8239

[rdstern]

@derekagorhom seems good except I get an error on the date. It seems to be pointing to a different variable, from the error message:

I look forward to the pull-down for the factor variable being re-instated as well as the logical variable. In each of those cases I hope NA will also be possible.

I agree we want all these to be working before merging.

[rdstern]

@derekagorhom this is coming on now, but still step-by-step.
a) With the date the default new date should start as the same as the original date.
b) You have added the NA as an option in the Logical variable. Well done. Can you do the same for a factor.
c) When I use it repeatedly it seems to remember more than it should from the previous use of the dialog. For example I checked and edited a logical column, and then tried a date. It wanted me to change into another logical setting, and not a date. I pressed reset and it looked very odd. I suggest reset should just set it correctly for the current cell?

[N-thony]

@rdstern i brought back the old selector because the new one was the reason the factor dropdown was not working and i also had a discussion with @N-thony and i realised that The receiver will always be highlighted since it is linked to the selector because that is how it has been done internally, we are still looking for ways to solve this. In the meanwhile i have already added the date control for the dialog and it work well now. Can this PR be kept open while i work on the logical option so that this can also close #8239

@derekagorhom we can have a quick call on this, I think I have a solution for the highlight of the receiver

[derekagorhom]

@rdstern After some discussions with @N-thony we decided to disable the reset button since it was not needed any more
we also managed to fix the remaining issues
can you review this again.

Thanks

[rdstern]

@derekagorhom I am afraid I found a problem below, when using R-Instat in a language other than English. But apart from that - you seem to have "nailed" it. I tried with numeric and factor and date. Oh, but now I found a new different problem in the English version - see below

@derekagorhom and @lloyddewit by chance I started with R-Instat in Portuguese. Here is the dialog when editing the variable fert, etc.

It was cell 3 and the value was 3. The items are just a bit too far left in each control.

Here it is in English:

And here in Russian? It isn't usable now:

I suspect that there is a left, rather than a right-limit on the controls you are using - or vice-versa?

Now the new problem I found: Here is a picture:

I moved data frame and - unusually in our dialogs this dialog should always "follow me". It should always relate to the current data frame, because that is the one I am editing. I don't know if that is simple for you, or you will need a bit of help to make this change efficiently.

But, apart from that I am very happy wiuth this feature now.

[rdstern]

@derekagorhom this is looking great now. Many thanks. I am approving. Later we may want to be able to call it alternatively from the Edit menu. For now, I am happy where it is!
@lloyddewit over to you.

[lloyddewit]

@derekagorhom thanks, looks good, just a few questions

[derekagorhom]

@lloyddewit I have made the changes and answered your comments, can you review this PR again
thanks

[lloyddewit]

@rdstern There have been some changes since you last tested. Please could you retest? Please could you pay special attention to the column receiver (to ensure that it is read-only when required and has the correct colour?
Thanks

[rdstern]

@derekagorhom this is still looking good. But @lloyddewit asked me particularly to look at the variable receiver. Here it is the first time I use Edit Cell:

But whenever I return to Edit Cell after the first time, it appears enabled - as so.

It isn't a major thing, because I don't think anything can be done - though I can delete it and then I can't make the change - see below:

And I can't reset, because that is disabled. So I think this needs to be fixed. I do hope we can include it in the next release.

[N-thony]

@rdstern @derekagorhom what if we simplify this by changing the Receiver to an InputControl and set it to ReadOnly?

[rdstern]

@N-thony I don't understand the implications of your suggestion. But it was greyed out, so disabled, the first time it is used, see above. It is my lack of programming skills, but why can't it stay like that thereafter?

[N-thony]

@N-thony I don't understand the implications of your suggestion. But it was greyed out, so disabled, the first time it is used, see above. It is my lack of programming skills, but why can't it stay like that thereafter?

@rdstern yes, I forgot to update my comment. This is the issue with control, the code is correct with ReadOnly applied to it but it is not, you can still delete a text on it. I'm looking at it.

[rdstern]

This has become a really nice feature to show how useful the grid can be. So I am keen to see this finished. Then I have lost touch with the state of the other improvements. Did anything happen on the wrap? And was there resurrection on the paste. Then I would like still to proceed on the Find rows (using filter) and find columns (using select)! There is quite a lot, still to do.

What I really like about the new Edit Cell, is that it links nicely now to helping users when there is data of different types - and (so far) numeric, character, factor, data and logical.

[N-thony]

This has become a really nice feature to show how useful the grid can be. So I am keen to see this finished. Then I have lost touch with the state of the other improvements. Did anything happen on the wrap? And was there resurrection on the paste. Then I would like still to proceed on the Find rows (using filter) and find columns (using select)! There is quite a lot, still to do.

What I really like about the new Edit Cell, is that it links nicely now to helping users when there is data of different types - and (so far) numeric, character, factor, data and logical.

@rdstern what I think it will be good for this PR to not be held up too long, is if you are okay with changes made, we cane merge it as it is for now. The issue with the receiver seems to be general because the code for it being enabled is well implemented but it is not working, I'm investigating on it and so far this can be an issue with the .Net framework version we are using. I will suggest that @derekagorhom might open an issue about.

[derekagorhom]

@rdstern the new changes solves all the issues raised in #8366 (comment)

can you review this.
Thanks

[rdstern]

@derekagorhom excellent. It now seems to work well.

The merge-base changed after approval.

The merge-base changed after approval.