XWIKI-7540: Merge Annotations with Comments. #34
Conversation
|
This is very cool Edy ! Any plan to have a migration script ? I think it would be mandatory at this stage if we were to introduce this merge. |
- Reverted the deletion and usage of AnnotationCode.Tab and annotationsinline.vm since they should still be available to people using custom annotations (that use a class other than XWiki.XWikiComments) - Hiding the Annotations docextra tab by default since it was replaced by the Comments tab
- Reverted the deleted annotations docextra shortcuts.
- Added migration script.
- Rewritten the migration script to a working and tested version.
- removed reply button from non-threadable annotations - annotations js now uses either the Comments or the Annotations tab, deppending on what annotation class is used - the delete listener for annotations in the Comments tab now registers only once (the listener from comments.js). The listener from AnnotationCode.Script now registers only for (custom) annotations in the Annotations tab. - fixed the problem with deleting an annotation from the Comments tab when annotations are not visible by marking the fetched annotations as dirty, to be refetched when the annotations are made visible -- the js method fetchAnnotations() now always replaces the content with the annotated content, even if no annotation is present (to catch the case when it is called from the Comments tab to delete an annotation -- the last annotation) - reverted the suppression of the JS that handled the content menu to follow the scroll, since we can use this.displayedByDefault when the this.displayAnnotationsCheckbox is not yet present in the document.
- small fix to not test for "AnnotationCode.AnnotationsClass", but to use "XWiki.XWikiComments" instead to allow the use of another annotation class.
|
Pull request updated (including migration script) and ready for review. |
There was a problem hiding this comment.
We're using Prototype 1.7 now. Is this quick fix still needed?
There was a problem hiding this comment.
Probably it is no longer needed, but that is outside the scope of this pull request.
- Removed the js hack for the annotation bubble reply button and, instead, reused the corresponding comments tab reply button's click listeners.
- Extended the fetchAnnotations() method with a 'force' parameter instead of assuming always 'true'. This should better preserve the other uses of the method while allowing the new behavior as optional.
There was a problem hiding this comment.
why do we need to change this? If I understand correctly from this diff, it makes annotations display by default on all the pages where they're enabled. Please correct me if i'm wrong and ignore my comment.
The thing is, the way the annotations get displayed right now is by replacing the content of the page, with the annotated content from the REST service.
This request for the annotated page content doesn't take into account GET nor POST parameters (it makes the request for the page itself) and doesn't re-launch xwiki:dom:loaded when it's done so that some javascripts don't re-execute (e.g. the javascript that creates the little edit icons for section editing under the headings).
As far as I know annotations are enabled everywhere by default, so this mean that you can never search: since the search string is passed in the get params and when the search results page loads it automatically reloads with annotations, you don't see it. There is indeed a check: if there are no annotations, the js doesn't replace document content, but if ever you put an annotation, the behaviour I just described will take place "by default" and there's no way to prevent it, unless you know the cause.
I don't know about this parameter: it's interesting when it's set, to have annotations by default but it has the problem I mentioned above. Maybe we could restrict some spaces by default, like the main, xwiki, the blog where it won't work, etc, since we're providing a customization of the annotations system anyway...
There was a problem hiding this comment.
No, this is a slip on my side. Thanks for catching it.
Will leave it on 0, as before.
Setting it to 1 is a different topic that we could discuss on the list.
- Renamed the migration script because the initial version is already taken. Next commit contains the modification of the script itself to reflect this change.
- Renamed the migration script because the initial version is already taken. (continued) - Renamed migration script internal class to exclude the xwiki db version from its name - Reverted the unintentional setting of anntations to enabled by default - Added extra js documentation - Made sure the "See thread" message is properly generated
- Some more comments with details for future improvements.
- Refactored the migration script to work on one document per transaction.
- Properly handling empty StringListProperty values in the migration script.
There was a problem hiding this comment.
Thanks. Fixed by removing leftovers.
Anything else?
- Minor cleanup of leftover and unused method parameters and updated class @SInCE javadoc.
…ments XWIKI-7540: Merge Annotations with Comments.
…-integration Update from Weblate
5 participants
No description provided.