-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Improve a few UI items and their corresponding documentation #1014
base: master
Are you sure you want to change the base?
Conversation
Please review the commits separately and I'll squash all when I commit. |
❌ Unit Tests, Quality checks, and Acceptance Tests failed. Please look a Gradle Scan page for details: |
I'll try to get started on this today, but my wife has plans to steal most of my free time while she's off today and tomorrow, so the bulk of the commits might not get done before Wednesday, when she's at work 😅 |
doc_src/en/OmegaT_Preferences.xml
Outdated
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.
The whole shebang about switching from the clear but long "project-specific" to a much more confusing but shorter "local" was the length of the string, IIRC. Now, "locally-defined" is only one character shorter than "project-specific", but still not nearly as clear.
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.
No, it was not the length of the string that brought the modification but the fact that the UI did not have a word for the "default" which is what we call now "global". And that was extremely confusing.
Now we have a clear opposition between "global" and "local" all over the UI.
This modification is not about relabeling all the "local" properties as "locally-defined" but just to remove the confusion that comes from having "local" next to "external" in one item. Maybe the second one is not necessary. And maybe there is a better way to fix what I think is confusing about having "local external things".
I'm fine if you oppose the modification. That's the reason why there is a review. But it was not a "shebang" that we had but quite a few thoughtful exchanges over the course of a few months and I don’t have the feeling that OmegaT is worse off with the modifications that we made to the UI and the extensive work that we did on the manual to clarify everything.
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.
My comment was in no way meant to diminish what had been done, or to object the change. We crossed that bridge, and clarifying things is always good. It was more of an amused general observation, no need to take it so seriously. Sorry for causing a stir-up.
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.
Don’t worry. I’m fine :-) If you think the changes are not necessary, please say so. I don’t mind removing that part of the proposal and I’m OK with proposals that help "unweirden" that label.
This comment was marked as resolved.
This comment was marked as resolved.
❌ Unit Tests, Quality checks, and Acceptance Tests failed. Please look a Gradle Scan page for details: |
Why does GitHub keep changing that single apostrophe if I commit and push from Intellij IDEA?
❌ Unit Tests, Quality checks, and Acceptance Tests failed. Please look a Gradle Scan page for details: |
src/org/omegat/Bundle.properties
Outdated
|
||
WF_OPTION_INSERT_AUTHOR_PREFIX=&Name/ID: | ||
WF_OPTION_INSERT_AUTHOR_PREFIX=Nickname: |
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.
I'm not entirely sure about "nickname". It's not wrong, but something about it bugs me a little, though I can't quite put my finger on why. Maybe "alias" would be better?
Given the context, I think line 1772 should probably be:
Enter the alias you want to use to identify your translations in the memory.
("Enter the nickname…" if we decide to keep that.)
Line 1774 then either becomes Alias:
or stays as is.
Interestingly, this alias/nickname applies even in non team projects, so this entire section should perhaps be under the General
heading, but that's beyond the scope of this specific PR.
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.
Alias is fine. You're correct about the location, but it was a "team" feature before the synchronization thing was implemented.
OK, I've gone through each individual commit and…
The last one simply means that particular commit is fine as is. On a slightly different note, I just noticed in looking at some of these tweaks that we have a mix of "directory" and "folder". We should probably pick just one of those. If memory serves, "directory" is favoured in Linux, and "folder" is preferred in Windows. Not sure which one macOS tends to use more. I don't have a strong preference either way, though I suppose that "folder" has the benefit of being a few characters shorter, if that's a consideration. (I haven't checked, but I have a hunch that we have more uses of "folder" than "directory", so keeping the former may be the path of least resistance.) |
Just noticed this comment. Indeed. We should remove that! |
@@ -56,7 +56,6 @@ private void initComponents() { | |||
java.awt.GridBagConstraints gridBagConstraints; | |||
|
|||
intervalDescriptionPanel = new javax.swing.JPanel(); | |||
intervalDescriptionTextArea = new javax.swing.JTextArea(); |
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.
You should not directly edit SaveOptionsPanel.java
but you edit it through NetBeans
GUI editor.
The change break SaveOptionsPanel.form
divert with generated Java code.
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 builds properly here. I'm OK with editing things in Netbeans but I asked how to use the GUI Editor in the past and nobody answered.
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.
I remember a question was too general, and I could not say RTBM in public ML for core team.
But I need to say readthedocs of Coding Styles which can be improved to explain clearer.
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.
I don’t know if that's the information I need, but that's the kind of link you could have posted when I asked for information.
Regarding the coding style file, we may need to improve it a bit but I need first to identify the reason why I was stuck.
Thank you for the reference.
<!-- Note: The OmegaT project uses GitHub pull requests for code review only.
The "real" code base is hosted on SourceForge; the GitHub project is a mirror.
If your PR is accepted it will be applied to the SourceForge repository by a
core contributor and the PR ticket will be closed. It will appear to have been
"closed without merging" but that is normal. --->
↑ maybe we can remove this part from the template now ?
What does this PR change?
Other information
We need to have a style for error messages, with indication of a subject, a verb, etc. so that have a better understanding of what is going on.