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.

Sign up for GitHub

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

Jump to bottom

Add image caption support #911

Merged
merged 12 commits into from
Feb 17, 2020
Merged

Add image caption support #911

merged 12 commits into from
Feb 17, 2020

Conversation

thet
Copy link
Member

@thet thet commented Jun 11, 2019 •
edited
Loading

In the TinyMCE Image dialog:

  • Adds an option for image captioning support from the description of the image ("Show Image Caption from Image Description") (uses an plone.outputfilters filter),
  • Adds an textfield for manual image captions,
  • Wraps the image in a <figure> tag and adds a <figcaption> if "Show Image Caption from Image Description" is not checked and the manual caption is present.
  • If no caption is present, only the image tag is added.

Re-adds image caption support, which was present until Plone 5.

Screenshot:
Screenshot from 2019-06-20 11-39-45

Merge:
#911
plone/plone.staticresources#30
plone/Products.CMFPlone#2887
plone/plone.outputfilters#36
plone/plone.app.upgrade#226

@mister-roboto
Copy link

@thet thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@coveralls
Copy link

coveralls commented Jun 11, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.7%) to 58.243% when pulling e7896b0 on thet-figcaption into 7b32d1b on master.

@thet thet changed the title add figcaption support A image caption support Jun 19, 2019
@thet thet changed the title A image caption support Add image caption support Jun 19, 2019
thet added a commit to plone/plone.staticresources that referenced this pull request Jun 19, 2019
@thet
Add figcaption support - plone/mockup#911
7486247
@thet thet force-pushed the thet-figcaption branch from abbd851 to ed6dbc7 Compare June 19, 2019 01:24
thet added a commit to plone/plone.staticresources that referenced this pull request Jun 19, 2019
@thet
Add figcaption support - plone/mockup#911
17e8b34
This was referenced Jun 19, 2019
Merged
Merged
Merged
@thet thet assigned iham and MrTango Jun 19, 2019
@thet thet force-pushed the thet-figcaption branch from ed6dbc7 to 17e42b9 Compare June 19, 2019 01:32
thet added a commit to plone/plone.staticresources that referenced this pull request Jun 20, 2019
@thet
Add figcaption support - plone/mockup#911
ee430b2
@thet thet force-pushed the thet-figcaption branch from c780d60 to d8d27de Compare June 20, 2019 08:30
@thet
Copy link
Member Author

thet commented Jun 20, 2019 •
edited by flipmcf
Loading

The PRs regarding image captioning support are almost ready.
But there are some open questions

  • plone.outputfilters.filters.resolveuid_and_caption.ResolveUIDAndCaptionFilter uses the IImageCaptioningEnabler utility to check if the image captioning filter should be applied. We do not register such a utility and we do not have a controlpanel setting for that. So that filter always returns false.
    In contrast to that, we have a ResolveUidsAlwaysEnabled in plone.outputfilters which always returns true, so resolving uuids works.
    AFAIK this is the core of the image captioning regression from Plone 4.
    Probably the captioning setting should go in the image controlpanel.
    Ref: https://github.com/plone/plone.outputfilters/blob/bf2c1d160039b83299abca5f9467fc51cf9ef884/plone/outputfilters/filters/resolveuid_and_caption.py#L41

  • plone.outputfilters uses a image captioning template which uses a dl/dd structure. This needs to be changed to figure/figcaption.

  • At the Buschenschanksprint we discussed potential accessibility problems. If we add a figcaption from the description, we need to omit the alt tag from the image.
    To fix that properly we would need to specify how images and image captions should be done in respect to accessibility.

Before proceeding I'd like to get some feedback.

@thet
Copy link
Member Author

thet commented Jun 20, 2019

Some more ideas (out of scope for this PR):

  • Image copyright info handling in plone.outputfilters.
  • Copying EXIF information when scaling.

@thet thet mentioned this pull request Jun 20, 2019
Closed
@thet
Copy link
Member Author

thet commented Jun 20, 2019

Just for reference: other issues and discussions regarding this topic:

@thet thet force-pushed the thet-figcaption branch from d8d27de to e7896b0 Compare June 21, 2019 13:30
@thet thet mentioned this pull request Jun 21, 2019
Merged
thet added a commit to plone/plone.staticresources that referenced this pull request Jan 3, 2020
@thet
Add figcaption support - plone/mockup#911
f4b289e
@thet thet requested review from jensens and esteele January 3, 2020 16:03
@thet thet mentioned this pull request Jan 3, 2020
Closed
@davilima6
Copy link
Member

@jenkins-plone-org please run jobs

thet added a commit to plone/plone.staticresources that referenced this pull request Jan 5, 2020
@thet
Add figcaption support - plone/mockup#911
dde0a6f
thet added a commit to plone/plone.staticresources that referenced this pull request Jan 7, 2020
@thet
Add figcaption support - plone/mockup#911
05e24f4
thet and others added 12 commits February 11, 2020 16:40
@thet
add figcaption support
0c5cb50
@thet
Preserve HTML formating in image captions
d60bdda
@thet
Use a textarea for the image caption
7d0419c
@thet
optional image captions
ba2fe86
@thet
add generic class image-richtext for accessing richttext-added images.
b61f32f
@thet
Add original image src for internal images or src for external images…
07252a5 
…. Can be used to open the image in a modal in a different resolution than the one which is linked.
@thet
We most probably do not need the data-src attribute. A pattern can sc…
7d4d091 
…an for all image-richttext images and split on the @@images view and get the original scale. Done.
@thet
Add tests for figcaption support.
cee6aba
@thet
update news file
7689904
@thet
Element.closest support is not widely available. Use $(element).close…
5350507 
…st instead.
@thet
remove the imagecaption option. just show image captions
3d5c57c
@davisagli @thet
Integrate caption from description support from d3d11d0
a4779b3
thet added a commit to plone/plone.staticresources that referenced this pull request Feb 12, 2020
@thet
Add figcaption support - plone/mockup#911
44a26b4
@thet thet merged commit 3e64876 into master Feb 17, 2020
@thet thet deleted the thet-figcaption branch February 17, 2020 14:03
@thet thet mentioned this pull request Feb 17, 2020
Merged
thet added a commit to plone/plone.staticresources that referenced this pull request Feb 17, 2020
@thet
Add figcaption support - plone/mockup#911
9cb61e0
thet added a commit to plone/plone.staticresources that referenced this pull request Feb 17, 2020
@thet
Add figcaption support - plone/mockup#911
05a12a6
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Feb 17, 2020
@thet
[fc] Repository: plone.staticresources
3581f2a 
Branch: refs/heads/master
Date: 2020-02-17T18:17:32+01:00
Author: Johannes Raggam (thet) <thetetet@gmail.com>
Commit: plone/plone.staticresources@05a12a6

Add figcaption support - plone/mockup#911

Files changed:
A news/30.feature
A src/plone/staticresources/upgrades/7.zcml
A src/plone/staticresources/upgrades/profiles/7/registry.xml
M src/plone/staticresources/profiles/default/metadata.xml
M src/plone/staticresources/profiles/default/registry/bundles.xml
M src/plone/staticresources/setuphandlers.py
M src/plone/staticresources/upgrades/configure.zcml
Repository: plone.staticresources

Branch: refs/heads/master
Date: 2020-02-17T18:17:32+01:00
Author: Johannes Raggam (thet) <thetetet@gmail.com>
Commit: plone/plone.staticresources@cad5e61

compiled

Files changed:
M src/plone/staticresources/static/plone-tinymce-compiled.js
M src/plone/staticresources/static/plone-tinymce-compiled.min.js
@thet
Copy link
Member Author

thet commented Feb 17, 2020

@iham all merged! Should be in the next Plone release.

This was referenced May 21, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jensens jensens Awaiting requested review from jensens

@esteele esteele Awaiting requested review from esteele

Assignees

@iham iham

@MrTango MrTango

Labels
02 type: regression 04 type: enhancement 13 prio: normal 24 status: ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@thet @mister-roboto @coveralls @davilima6 @iham @MrTango @davisagli