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

Added ability to change placeholder text for comment/tag, and make comment replies optional #113

Merged
merged 2 commits into from
Oct 2, 2022

Conversation

ahammouda
Copy link
Contributor

I would like to be able to disable the replies in the comment widget without actually disabling comments for a smoother experience were annotation tasks will not involve collaboration.

Specifically I'd like to make Annotorious initialization to optionally be invoked as follows (modified from this example):

      window.onload = function() {
        anno = Annotorious.init({
          image: 'hallstatt',
          locale: 'auto',
          drawOnSingleClick: true,
          widgets: [
            { widget: 'COMMENT', disableReply: true  },
            { widget: 'TAG', vocabulary: [ 'Animal', 'Building', 'Waterbody'] }
          ]
        });

Again, you get similar behavior with the readOnly flag, but then a user cannot modify the comment at all. Alternatively a user could simply use the tag widget with an unfixed vocabulary for this purpose, but there are still some use cases where that's not ideal. What's more you can't actually add a custom placeholder to indicate a different kind of annotation task - that could also be a reasonable PR to make if it was desired

For example, you may want to annotate some OCR by both identifying the part of speech (from a fixed vocabulary described in the tag widget), as well as correct any misinterpretations of the read text (via the 'comment' widget).

The final thing added in this PR is the ability to change placeholder text for either the comment or tag widget. Making the final instantiation look like:

      window.onload = function() {
        anno = Annotorious.init({
          image: 'hallstatt',
          locale: 'auto',
          drawOnSingleClick: true,
          widgets: [
            { widget: 'COMMENT', disableReply: true, placeHolderText: 'Enter correct text'  },
            { widget: 'TAG', vocabulary: [ 'Animal', 'Building', 'Waterbody'], placeHolderText: 'Select the Appropriate Label(s)' }
          ]
        });

All these new fields are totally optional, and should change any existing behavior of the tool. Finally, my apologies for the additional whitespace changes - my text editor automatically removes trailing whitespace which seems like a decent enough practice - I hope it's not too distracting.

Copy link
Member

@rsimon rsimon left a comment

Choose a reason for hiding this comment

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

Thanks for this PR. That's a really useful addition!

Just one thing: I added a comment about the duplicate markup block, which I don't think is necessary, and could be simplified.

And another small issue: I'm not sure I'm convinced about the placeholder attribute. There is already a (global) way to override any label. It's not very visible in the docs, I admit. But in principle it's documented here:

https://recogito.github.io/annotorious/api-docs/annotorious/#initialization

See the messages init parameter. In your case, you could override with

messages: {
  'Add a comment...': 'My custom placeholder',
  'Add a reply...': 'My custom placeholder #2',
  'Add a tag...': 'My custom placeholder #3'
}

However, your placeholder argument doesn't cause any problem and might be a useful shortcut. I'm in inclined to leave that in.

src/editor/widgets/comment/CommentWidget.jsx Show resolved Hide resolved
@ahammouda
Copy link
Contributor Author

@rsimon - Thanks for the quick response/feedback. I didn't think to use the messages object - I'm happy to play around with that and see how it feels. Let me, know and I'll update the PR.

@rsimon rsimon merged commit 9c8810e into recogito:main Oct 2, 2022
@rsimon
Copy link
Member

rsimon commented Oct 2, 2022

Excellent, thanks for the quick correction! As I said: the placeholder prop definitely doesn't break anything, and adds minimal extra code. I'd vote for keeping it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants