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

XWIKI-9759: The javascript for the "Display annotation by default" feature gets cached too eagerly #2920

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

manuelleduc
Copy link
Contributor

@manuelleduc manuelleduc commented Feb 22, 2024

Jira URL

https://jira.xwiki.org/browse/XWIKI-9759

Note: as I wrapped a large piece of code in a require it is advised to review with Hide whitespace=true.

Changes

Description

  • Remove parse=true from AnnotationCode.Script, as a consequence:
    • All the translations are now loaded using a l10n Javascript module
    • the dependency to fast-diff is declared using a new org.xwiki.platform.requirejs.module UIX
    • the config is provided through a org.xwiki.platform.template.header.after UIX, providing the config in the form of serialized json in a #annotation-config script tag
    • the url to AnnotationCode.Settings in view mode is now provided to the config instead of being computed in the jsx
    • all the config initially computed in Velocity at the top of the jsx are now accessed from the config

Executed Tests

mvn clean install \
  -pl :xwiki-platform-annotation-ui \
  -f xwiki-platform-core/xwiki-platform-annotation \
  -amd \
  -Pquality,integration-tests,docker

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • N/A

@manuelleduc manuelleduc added backport stable-15.10.x Used for automatic backport to 15.10.x branch. backport stable-16.1.x labels Feb 22, 2024
@manuelleduc manuelleduc removed backport stable-15.10.x Used for automatic backport to 15.10.x branch. backport stable-16.1.x labels Feb 29, 2024

var XWiki = (function (XWiki) {
require(['xwiki-l10n!xwiki-annotation-messages'], function(l10n) {
XWiki = (function (XWiki) {
Copy link
Member

Choose a reason for hiding this comment

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

Why keep the anonymous function? Its purpose was to create a namespace so that we don't pollute the global namespace, but the require callback function does this now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But don't we still want to expose XWiki.Annotation publicly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, now I get what you mean, indeed I can drop a level of wrapping. Thanks!

@manuelleduc manuelleduc marked this pull request as draft October 11, 2024 07:31
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