-
Notifications
You must be signed in to change notification settings - Fork 103
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
Improved dialog layouts for French text in dialogs from Describe menu, Model menu and other menus #6580
Improved dialog layouts for French text in dialogs from Describe menu, Model menu and other menus #6580
Conversation
I have now finished checking the French layout of the Describe menu, except for the Plot Options dialog which we agreed to do at the end as it is very complicated. The excel document with comments about the translation Finally I am not sure if this is normal but in Multivariate-Principle Component Analysis-PAC Options dialog the save tab was completely empty. This seemed odd so I thought I should mention it. |
I have also finished the Model menu. I was not able to access the Display Options subdialog, but I am aware this has some layout issues and have created an issue to try to access this. I also found that a number of items in the Tidy submenu could not be opened (Other (one variable, Other (two variable...) Please find the updated excel document attached. |
@rachelkg Thanks. Is this PR now ready for test/review/merge or do you still expect to make additional changes? |
@lloyddewit I would like for it to be tested/reviewed and merged if possible. I will make changes as I receive comments from the review. |
@rdstern Please could you test? @shadrackkibet These are layout changes so in theory only the 'designer' and 'resx' files are changed. In practice, some of the dialog class files are also changed because the Winform editor sometimes adds empty handlers. @rachelkg is changing many dialogs so it will be time consuming to remove these empty handlers again before merging. |
@lloyddewit I am volunteering to remove all the empty handlers. Once ready I can pull these changes and work on it and merge them into this PR or branch it through another PR. |
@shadrackkibet Thank you for the offer. If we remove the handlers form this PR then we should also remove them from PR #6557 and PR #6558. This could be significant work. |
@lloyddewit @shadrackkibet Is it something I am doing wrong in my process that is creating all these ‘empty handles’. Please say if I could be doing something differently moving forward. |
@rachelkg This has to do with Visual Studio, sometimes it adds handlers automatically when you make designer changes. I would suggest that you always check your changes in VS before committing. Since you are only doing layout changes then you should ensure that there are no If you see any I hope your VS setup looks similar to mine above and hopefully, this sheds some light. |
@shadrackkibet Thanks for the explanation. |
@shadrackkibet I just noticed that empty handlers have also been added to PR #6543 . |
Thanks, @lloyddewit, I'm going to remove them all. |
@lloyddewit @shadrackkibet Thanks for the explanation. I will look out for these moving forward. Sorry to have created extra work for people! |
@rachelkg Nothing to apologize for, you couldn't have known this and we didn't warn you! |
Hi, I have now finished checking the French layout of the menus, except for the Import dialog, The Calculator dialog and the Plot Options dialog as they are either complicated and need further input or people are working on them. Please find the related excel document attached. There are a few outstanding issues but I think it is ready to be checked and merged. Thanks, Rachel |
@rachelkg Thank you for all your work! Before we merge, @shadrackkibet or I will remove the empty handlers. |
This could become long as I capture the screens sometimes to show a problem: While here I looked also at the summaries. (The "master copy" is possibly with Prepare > Reshape Data > Column Summaries) Describe > One Variable > Graph. Just 2 trivial points. Can the Describe > One Variable > Frequencies Minor changes also needed in the English as well. The label should be Continued below |
@rdstern thanks for checking through this and all your comments. I am away this week but look forward to reviewing and fixing everything up when I get back. I will try to have a work day early in the week probably the 27th. |
Hi @N-thony, Would you be able to look into resolving these conflicts please? @rdstern is going to discuss merging these with @shadrackkibet but if this could be done first that would be great. Thanks, Rachel |
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.
@shadrackkibet approved as explained on @rachelkg other pull requests
I am afraid this PR now has conflicts in many files. Fixing conflicts in |
@rachelkg @shadrackkibet I removed the unecessary handler subs that were accidentally added to this PR. I used the commands below. For more explanation, please see PR #6557
Note: I also had to undo some other changes in order to fix compilation errors:
|
@shadrackkibet I agree. The only way to resolve the conflicts would be to manually edit the conflicting files. However, the conflicting files are automatically generated and editing them would be dangerous. I don't think we should do this. @rachelkg I'm afraid that you'll have to implement your changes to the conflicting dialogs and subdialogs again. Future considerations: The (sub)dialogs affected are:
|
Hi @lloyddewit and @shadrackkibet, Thanks for all your work on this.. Sorry it have got so messy. I have made a note of the dialogs and subdialogs that need to be redone and will do those this week. Please feel free to go ahead with the merge. re: "Future considerations: I understand that you mainly work on Fridays. If possible, each Friday, you could try and make a new PR, that's ready for review. We could then merge it very quickly. This would reduce the risk of merge conflicts." Thanks for this suggestion I will do this moving forward. I am going to try to work on Tuesdays and Wednesdays over the summer so will make PRs on Wednesdays. Thanks, Rachel |
@lloyddewit just a question, I'm not sure and want your view. This PR has the changes up to 125 files, so it is a lot of work. I followed what you have done on PR #6557 about rolling back the affected files to the state they were in when PR #6557 was first branched. This effectively removes the changed files from PR #6557. So, is it possible to do the same here so that the files creating conflicts will be removed in this PR? Thanks. |
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.
@lloyddewit can this now be merged, before the latest one-way frequencies one? (#6657)
I have now redone the changed to the conflicting dialogs in a new pull request, which I will put up at the end of the day. Please let me know if there is anything else you need from me to get this merged. Thanks, Rachel |
1 similar comment
I have now redone the changed to the conflicting dialogs in a new pull request, which I will put up at the end of the day. Please let me know if there is anything else you need from me to get this merged. Thanks, Rachel |
I used the following Git commands: $ git merge-base master pr/6558-french-translation-updates-prepare-menu-up-to-view-labels-levels $ git checkout b291378 -- dlgBarAndPieChart.Designer.vb dlgBarAndPieChart.resx dlgBoxPlot.designer.vb dlgCircular.Designer.vb dlgColumnStats.Designer.vb dlgColumnStats.resx dlgHistogram.designer.vb dlgHistogram.resx dlgHistogram.vb dlgMerge.resx dlgOneVariableGraph.resx dlgOneWayFrequencies.resx dlgOptions.resx dlgRandomSample.resx dlgRestrict.resx dlgSummaryTables.Designer.vb dlgSummaryTables.resx sdgCorrPlot.resx sdgMissingOptions.Designer.vb sdgPlots.resx
I removed the conflicting files from this PR with the following git commands: $ git merge-base master pr/6580-french-layout-checked-for-the-describe-menu-onwards |
@shadrackkibet Please could you approve and merge? thanks |
I have been looking through the describe menu. I have checked up to the end of the Specific submenu but will continue working on the rest next week. Unresolved issues have been noted in the attached excel document.
Thanks,
Rachel
R-Instat translation changes.xlsx