omeka / plugin-ItemRelations

Allows administrators to define relations between items.
https://omeka.org/classic/plugins/ItemRelations/
7 stars 23 forks source link

Visual item ID selector / translatable relation texts + German translation #9

Open GerZah opened 9 years ago

GerZah commented 9 years ago

This pull request contains two things: (a) Visually selecting a relationship item instead of having to enter its ID. (b) German i18n, including the relationship terms (which were missing from the i18n file).

(--- Original pull request message ---)

While noticing that there was no German translation of plugin-ItemRelations yet, I also realized that all the relation texts could noch be i18ned, plus a handful of tiny text portions were also missing for a complete translatable version. This pull request contains both the additional i18n text portions in template.pot, the necessary tiny changes in config_form.php, formal_vocabularies.php, and ItemRelationsPlugin.php – and a complete German translation in de_DE.po / de_DE.mo. (Translation by me, might be more or less accurate at times, RFC.)

cokernel commented 9 years ago

@GerZah, thanks for writing this! This provides a nice interface for marking item relations, and I hope it will be merged.

GerZah commented 9 years ago

Thank you for the flowers. :-D

Am 17.06.15 um 16:59 schrieb Michael Slone:

@GerZah https://github.com/GerZah, thanks for writing this! This provides a nice interface for marking item relations, and I hope it will be merged.

— Reply to this email directly or view it on GitHub https://github.com/omeka/plugin-ItemRelations/pull/9#issuecomment-112833762.

Gero Zahn, gerozahn@campus.uni-paderborn.de University of Paderborn

zerocrates commented 9 years ago

You've alluded to the scaling issues with this approach in some other places, and I think that those alone would stand in the way of getting this merged. In short, a fetchAll for all items is a dealbreaker for me.

The translation parts are of course different. If you separated them into a PR for that alone, I wouldn't think there'd be any problems merging them (though we'd probably also ask you to hold the actual German translation and submit it through Transifex rather than here).

GerZah commented 9 years ago

Thank you for your feedback.

I know that fetchAll calls plus in-PHP JavaScript array encoding can lead to disaster. I'd definitely want to decouple the fetching of item titles / IDs from the main PHP call -- this begs for an AJAX responder, based on the incremental text input plus the object type selection.

However, I needed a solution that was ready to use quite quickly, so I allowed myself this quick & dirty solution. But I definitely have it on the map to change this -- I'm planning to have the incremental transfers carried out by my new student assistant who claims to be able to do PHP/AJAX/jQuery. Let's see about that. ;-)


As for the translations -- I understand that Transifex is fee-based, so I'd actually have to pay to contribute. Is that correct?

Unfortunately, at the moment this seems to be out of the question. Unless there's some educational plan at Transifex that is free for university students and stuff. Sorry!

Am 17.06.15 um 19:07 schrieb John Flatness:

You've alluded to the scaling issues with this approach in some other places, and I think that those alone would stand in the way of getting this merged. In short, a |fetchAll| for all items is a dealbreaker for me.

The translation parts are of course different. If you separated them into a PR for that alone, I wouldn't think there'd be any problems merging them (though we'd probably also ask you to hold the actual German translation and submit it through Transifex rather than here).

— Reply to this email directly or view it on GitHub https://github.com/omeka/plugin-ItemRelations/pull/9#issuecomment-112878884.

Gero Zahn, gerozahn@campus.uni-paderborn.de University of Paderborn

GerZah commented 9 years ago

Oops, the second part seems to have been cut off, so here once more:

As for the translations -- I understand that Transifex is fee-based, so I'd actually have to pay to contribute. Is that correct?

Unfortunately, at the moment this seems to be out of the question. Unless there's some educational plan at Transifex that is free for university students and stuff. Sorry!

zerocrates commented 9 years ago

Transifex doesn't cost anything to use, at least not the way we use it. You should be able to sign up for a free account and translate any of the resources we've posted there on our project page.

Basically, the process is that we'd accept improvements to what strings are translatable and the template, then push that template up to Transifex where all the translators can easily contribute.

GerZah commented 9 years ago

Oh, that's good to know. I promise to check it out.

Am 18.06.15 um 16:13 schrieb John Flatness:

Transifex doesn't cost anything to use, at least not the way we use it. You should be able to sign up for a free account and translate any of the resources we've posted there on our project page https://www.transifex.com/projects/p/omeka/.

Basically, the process is that we'd accept improvements to what strings are translatable and the template, then push that template up to Transifex where all the translators can easily contribute.

— Reply to this email directly or view it on GitHub https://github.com/omeka/plugin-ItemRelations/pull/9#issuecomment-113169322.

Gero Zahn, gerozahn@campus.uni-paderborn.de University of Paderborn

zerocrates commented 8 years ago

Translating the names/descriptions of the elements directly seems like the wrong path to me. Usually in this kind of situation, we translate strings like that "on the way out", only at display time. The way it's happening in this plugin, though, changes the actually stored data in the database, which seems likely to cause problems with interoperability, changing languages, or just updated translations.

This is the intention behind the "base" translation file: allow translatable strings to be manually specified in situations where you don't want to actually call __() on the source string directly. For example, the Omeka core's base POT contains the built-in labels and strings for the default shipped elements, so we can allow them to be translated, but don't actually modify the stored strings themselves.

Normally it'd be a little bit of a pain to generate the base POT because there's not great tooling for getting at those strings beyond xgettext (we actually have a pretty workable comment-based solution that's working for us pretty well in Omeka S but that's beside the point). Fortunately, you already have them all in your generated POT file, so you could just cut them out of there and put them into the base file, then remove the translation calls at the places where those elements and vocabs are defined and move them to the much smaller set of common places where they're displayed.

I'm not totally sure this would really cause an issue given that the data really seems specifically intended just for the plugin's internal use, but it seemed worth pointing out.

zerocrates commented 8 years ago

On a less technical note: I know this has been pending for a long time and we haven't been the most responsive about it. It can be hard to find time to devote to plugins when they're not otherwise being worked on, but we really do want to encourage these kinds of contributions and improvements.

I'd like to take this opportunity to make a gentle nudge in the direction of smaller, more focused pull requests. By now, this single PR contains translation support for the UI, translation support for the vocab and relation data, relation comment support, an item selection UI, various code style changes, search support, and likely more that I'm not immediately seeing or remembering.

I recognize that a big portion of this aggregation is our/my fault for letting this stew so long so that it represents such a large amount of time and effort in different dimensions of the plugin, but having so many unrelated or tangentially related things come in together for review makes it hard to merge any part of it.

There are plenty of people that can attest to our, let's say, spotty track record for more focused pull requests also, but we stand a lot more of a chance of making headway when we can easily do it piecemeal. We can sometimes do that with cherry picking, but that's not always a great solution, particularly when the history is long and complex and things are building on each other.

GerZah commented 8 years ago

Well – when we started to use ItemRelations within our German installation of Omeka, we were astonished to find all the relation titles and descriptions to be English only, without even the chance to translate them. Once I looked at the code, I figured out that the reason was plain and simple: The relations were inserted straight from untranslatable strings inside the install method.

So I ended up modifying these strings to become translatable. Granted: Their translations will now be imported only in their i18n-ed version when freshly installing the plugin (or uninstalling / reinstalling).

Daniel-KM commented 8 years ago

Hi,

Here, the question is Will the pull request be merged if the German issue is cleaned? But, in fact, the question is What about the role of the community in Omeka?

Sincerely,

Daniel Berthereau Infodoc & Knowledge management

GerZah commented 8 years ago

We ended up using our own relations, actually multiple vocabularies (we invested a bit of time to support multiple user defined vocabularies, have them editable, and whatnot). So if it's an issue to remove our __()-ed pre-defined relationship titles - do so, I don't really care. But still, I think they should be part of the .pot on Transifex to be i18n-ed if desired.

Gero Zahn, gerozahn@gerozahn.de, http://www.gerozahn.de/

Am 21.07.2016 um 22:13 schrieb Daniel Berthereau notifications@github.com:

Hi,

Here, the question is Will the pull request be merged if the German issue is cleaned? But, in fact, the question is What about the role of the community in Omeka?

Sincerely,

Daniel Berthereau Infodoc & Knowledge management

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

zerocrates commented 8 years ago

Will the pull request be merged if the German issue is cleaned?

By itself? No. As everyone allows, it's a big big change, and by accepting the PR we vouch for and become responsible for all of it. It's a lot to look through, but there's a significant amount of stuff that I'm concerned about. It helps to be specific so some of that is:

Some of this is minor or personal/institutional preference but others are things I wouldn't be comfortable shipping or having as an example (despite the obviously stellar quality of our documentation, users are often looking to learn by example).

So if it's an issue to remove our __()-ed pre-defined relationship titles - do so, I don't really care. But still, I think they should be part of the .pot on Transifex to be i18n-ed if desired.

I wouldn't suggest that the ability to translate that text be removed, just that it could be adjusted to perhaps avoid some issues.

But, in fact, the question is What about the role of the community in Omeka?

A little bit of a tougher question...

I'd say our goal is to have an open relationship with both the user and developer communities, whether that takes the form of informal support, issues, totally new plugins and themes, or pull requests. We're often falling short of this goal, for pull requests in particular, and for that I apologize. Even if we have issues with or simply won't accept a PR, people who've taken the time and effort to contribute deserve at least a prompt response saying so.

Practical concerns of time and other priorities get in the way, as well as frankly there being enough people with familiarity and interest in something like a plugin. Even then, our views or opinions on what's desirable or acceptable might not line up with whoever's looking to contribute a change. It's better to me to reject something, even if it represents a great feature or a lot of effort, than to accept something I don't fully understand or that I'm not ready to stand behind.

As much as possible I'd like to avoid the scenario of somebody just maintaining an ongoing fork if they're truly interested in upstreaming their changes. It duplicates effort and confuses the users. But, when we let things go as long as we have in some of these cases, we tend to reach a familiar problem: we have trouble dealing with the sheer mass of changes, and even when and if we do give them the necessary attention to provide commentary and critique, the contributors often understandably have little interest in making significant changes in something they already have working.

We need to do better from the Omeka team side of things, but external contributions are always going to be a little bit of a dialogue (or negotiation, if you prefer).