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

New image viewer #6035

Open
wants to merge 25 commits into
base: 6.2
Choose a base branch
from
Open

New image viewer #6035

wants to merge 25 commits into from

Conversation

Cyperghost
Copy link
Contributor

@Cyperghost Cyperghost commented Oct 23, 2024

Closes #5966

TODO

  • Remove language variables wcf.imageViewer.*
  • Check that .jsImageViewer is no longer used
    • Check that data-caption and not title is used
  • Use shared_imageViewer instead of imageViewer
  • Rebuild WCF.Combined.min.js and WCF.Combined.tiny.min.js
  • Add documentation how to use the new image viewer (6.2 Document new Image Viewer docs.woltlab.com#480)
    • Add migration guide from old image viewer

Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

The new library should be added to

name: "Check that npm dependencies are in sync"

@dtdesign
Copy link
Member

Note: This is blocked while we’re discussing the licensing terms with the vendor.

Copy link
Member

@dtdesign dtdesign left a comment

Choose a reason for hiding this comment

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

You forgot to update files/js/.buildOrder, it still includes WCF.ImageViewer.

Also, are we somehow able to dynamically group images that exist in the same message? Currently the image viewer spans all images on one page which also means that, for example, attachments from multiple posts are mixed together.

com.woltlab.wcf/templates/shared_imageViewer.tpl Outdated Show resolved Hide resolved
com.woltlab.wcf/templates/shared_bbcode_wsm.tpl Outdated Show resolved Hide resolved
ts/WoltLabSuite/Core/Component/Attachment/Entry.ts Outdated Show resolved Hide resolved
ts/WoltLabSuite/Core/Bootstrap.ts Show resolved Hide resolved
Copy link
Member

@dtdesign dtdesign left a comment

Choose a reason for hiding this comment

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

Looks good so far.

Please do not merge it yet, I will have to make some adjustments and conduct some further tests first.

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.

4 participants