[drupalMedia] [drupalImage] Make image title attribute configurable

Created on 9 August 2023, 11 months ago
Updated 12 February 2024, 5 months ago

As the title states, it would be nice to make the title attribute of images configurable with drupalMediaInsert within ckeditor5.
There are multiple reasons why this is an important feature, the title text will be displayed on hover and also have an impact on SEO.

I already made some work that does itยดs job for drupalMedia, i made it analogue to the MediaTextAlternative plugin. We needed this functionality urgently after switching from ckeditor4 and using entity-embed for media to ckeditor5 and migrating to drupal-media.

Remaining work:


- It adds the title to the allowed elements and therefore some kind of updatehook is required (or disabling and enabling the media-embed fitler)

I will add the code after I created the issue

โœจ Feature request
Status

Needs work

Version

10.1 โœจ

Component
CKEditor 5ย  โ†’

Last updated 11 minutes ago

Created by

๐Ÿ‡ฉ๐Ÿ‡ชGermany sascha_meissner Planet earth

Live updates comments and jobs are added and updated live.
  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs accessibility review

    Used to alert the accessibility topic maintainer(s) that an issue significantly affects (or has the potential to affect) the accessibility of Drupal, and their signoff is needed (see the governance policy draft for more information). Useful links: Drupal's accessibility standards, the Drupal Core accessibility gate.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @sascha_meissner
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany sascha_meissner Planet earth
  • Status changed to Needs review 11 months ago
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany sascha_meissner Planet earth

    I also added the functionality to drupalImage, for me everything is working pretty good, please review my changes in the 3380223-mediainsert---make branch :-)

    Whats still open is that the allowed_html changes by adding the title attribute, iยดm not sure how to update this and need help whats the best way to do so. Also, as this functionality is missing in ckeditor5 library it might make sense to port it there.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany sascha_meissner Planet earth
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany sascha_meissner Planet earth
  • Open on Drupal.org โ†’
    Environment: PHP 8.2 & MySQL 8
    last update 11 months ago
    Not currently mergeable.
  • Status changed to Needs work 11 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Opened the MR so it may reviewed.

    Have not reviewed it myself but something like this I can see will need test coverage.

    Thanks

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany sascha_meissner Planet earth

    Hey @smustgrave, thanks for your reply. Surely i will update/adjust the tests for media to also check for the functionality, but iยดd say before that the actual code change should be reviewed and in this process probably further developed. Itยดs the first time I try to contribute something to core, especially using MRยดs so please go easy on me :) I actually dont know why the branch is currently not mergeable, i dont know where i find any output about it. But yes iยดd love to write the tests as soon as the feature is finalized.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany sascha_meissner Planet earth

    One thought i have is if there should be a default text for the title attribute, maybe it makes sense to use the text-alternative? Ideally this would be configurable, on most of our systems, in example, medias have a title field with a designated title additionally to the text-alternative. This could possibly be a setting within the media tab in text-format edit page?

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany FeyP

    @sascha_meissner Thanks for working on this. The merge is blocked because there are new commits on 10.1.x and your branch has diverged, so a fast-forward merge is not possible. To fix this, you need to rebase the MR branch. You can see this when you open the MR page in GitLab:

    > Merge blocked: fast-forward merge is not possible. To merge this request, first rebase locally.

    See here for more info: https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... โ†’

    To get an early review or a second opinion for design decisions, you could ask in #ckeditor5 or #contribute in Drupal Slack.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany sascha_meissner Planet earth

    Itยดs been some time ... just rebased origin/10.1.x into my issue fork (which was a little nasty because i needed to rebuild ckeditor5 for every commit :P ) Also didnt manage yet to join the slack or do any other progress related to the topic, at least you can patch 10.1.8 now with the diff

  • Pipeline finished with Failed
    5 months ago
    #82320
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium Wim Leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Epic work here! ๐Ÿ‘

    1. This is modifying DrupalInsertImage too? Would this title be more accurate? Can you update the issue summary to explain why we should modify both simultaneously in this issue?
    2. Also: I recall that the title attribute can be problematic wrt its impact on accessibility. Can you also explain in the issue summary why this would not be a concern?
    3. Finally: I see no test additions nor changes ๐Ÿ˜… There is a lot of test coverage for related functionality, expanding test coverage should be relatively straightforward.
Production build 0.69.0 2024