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

Glossaries change layout #39

Closed

Conversation

t-cordonnier
Copy link
Contributor

Enable to:

  • set colors associated to source, translation and comment of glossaries
  • choose layout between actually 4 layouts
  • define new layout classes as plugins, which can be loaded from external JAR

Copy link
Member

@amake amake left a comment

Choose a reason for hiding this comment

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

Sorry for the very, very long delay on this.

If you're still willing to work on this with me, I have some questions.

src/org/omegat/core/Core.java Outdated Show resolved Hide resolved
src/org/omegat/gui/glossary/IGlossaryRenderer.java Outdated Show resolved Hide resolved
src/org/omegat/gui/glossary/GlossaryTextArea.java Outdated Show resolved Hide resolved
src/org/omegat/gui/glossary/TransTipsMarker.java Outdated Show resolved Hide resolved
@amake
Copy link
Member

amake commented Dec 28, 2020

Thanks for updating the PR and for your answers.

I'm worried that if we have too much back-and-forth then I'll run out of time and this will drag on even longer. So I'm going to make the changes that I'd like to see and then push them for discussion. I'll let you know when I'm done.

@amake
Copy link
Member

amake commented Dec 28, 2020

I've made my changes.

  1. I moved storage of IGlossaryRenderers to the GlossaryRenderers class

  2. I changed the built-in renderers to no longer be plugins.

    We really need to be able to assume that DefaultGlossaryRenderer exists. DictionaryGlossaryRenderer could be a plugin, but I don't see a need for it to be one right now.

  3. I made merging/not-merging a separate option

  4. I reverted the default glossary colors because I found them too garish.

    I would be OK with changing the defaults to something not too far from the current scheme, for instance making the "note" a shade of gray to de-emphasize it a bit. But I feel pretty strongly that bright colors need to be opted into by the user. I also suspect that there is not going to be a consensus on what colors should be used. That means that users who want color currently need to manually customize the colors, and I'm OK with that.

Please let me know what you think.

@t-cordonnier
Copy link
Contributor Author

1. I moved storage of `IGlossaryRenderer`s to the `GlossaryRenderers` class

OK for me.
I did a last change in this class: since GlossaryTextArea and TransTipsMarker do very frequent calls to the method getPreferredGlossaryRenderer(), better do not browse the list each time.

2. I changed the built-in renderers to no longer be plugins.
   We really need to be able to assume that `DefaultGlossaryRenderer` exists. `DictionaryGlossaryRenderer` could be a plugin, but I don't see a need for it to be one right now.

OK for me

3. I made merging/not-merging a separate option

Yes. See my comment about this point in a separate message.

4. I reverted the default glossary colors because I found them too garish.
   I would be OK with changing the defaults to something not too far from the current scheme, for instance making the "note" a shade of gray to de-emphasize it a bit. But I feel pretty strongly that bright colors need to be opted into by the user. I also suspect that there is not going to be a consensus on what colors should be used. That means that users who want color currently need to manually customize the colors, and I'm OK with that.

That is your choice, since it is now configurable it can be changed later, so no problem for me.
I don't know how I could have choosen less "garish" colors, except, as you say, with some shades of gray. Here is how I did the choice:
a) Be consistent with other colors used in the application. My idea was to use the same colors as the ones used in the editor (blue for source, green for translation, purple for notes). But now I realize that DGT did not use the same color scheme as standard OmegaT and I forgot to replicate the change.
b) Since in the editor we use background colors while the glossary pane uses font colors over white background, I must use darker colors (reason why I use "dark green", because standard green seems difficult to read on white screen)
c) Use standard colors, such as Color.blue, instead of a self-made combination, so that they don't immediately appear as strange for the users

What you say about consensus is not necessarily false, but it did not prevent from having lot of colors in the past in the editor, so I think a discussion should be opened in the users list. That said, we can commit like that and change later if a consensus arrives.

@amake
Copy link
Member

amake commented Dec 30, 2020

Thanks for your help and your patience with this PR. I've squashed and pushed to master on SourceForge.

I'll be happy to revisit details based on feedback.

@amake amake closed this Dec 30, 2020
Kazephil pushed a commit to Kazephil/omegat-kaze that referenced this pull request Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants