🇬🇧United Kingdom @jacobupal

Updated the issue branch to make it merge-able again since conflicts arising from https://www.drupal.org/project/drupal/issues/3396165 📌 [meta] Convert all core plugin types to attribute discovery Active

Just to illustrate, I performed the same 3 steps and have screenshot what happens before and after this proposed change.

The steps:

1. User "jacob" creates the new article.
2. User "clare" makes a change to the article.
3. User "admin" performs a replacement which affects the article.

This first image compares two screenshots; before and after. It can be seen that there is no change to the "author" from this proposal.

However, when viewing revision history there is a difference.

Before the patch/MR, we can see the latest is revision is incorrectly attributed to "clare":

After the patch/MR we can see the latest revision is properly attributed to "admin":

I think you might have misunderstood (forgive me if not!). This proposal would not make the person running the replacements the author of the piece itself (the "uid") instead I'm proposing that the "revision_uid" value (not "uid") in the new revision should reflect who ran the replacement.

In your example, the author of piece would remain whatever it was before the replacement, but when reading through the revision history, the change would be now attributed to the administrator who ran the replacement, this would only be visible by users who can read the revision history. Or if for example you have made the revision user visible in your theme using some view/twig like "Created on [created] by [uid] and last updated on [changed] by [revision_uid]".

With the current behavior: if I make a normal change to an article, and a new revision is made, that change/revision is always attributed to me by default. But then, if somebody runs a bunch of replacements after me, using this module, those changes are now attributed to me too, even though I have nothing to do with them. Those new revisions are not my work, and if somebody asks me why I made those changes I won't have anything to tell them.

I can't really think of a situation where this proposal shouldn't be the default, because that is who made this change. Attributing changes/revisions to people who didn't make those changes seems like a bug to me...

I can see a situation where you might like the option want to force "revision_uid" to be some generic user and call it "bulk changes" or something, however I think that'd be a different proposal/issue.

xjm → credited jacobupal → .

jacobupal → created an issue.

jacobupal → created an issue.

jacobupal → created an issue.

I've been playing round with the CSS for the same reason and it seems there's no way of adding a color overlay to an image without writing js or changing the markup... the issue is that img tags are self-closing so you can't use pseudo elements.

What did work was adding outlines though which I think helps at least show which version version of the image was deleted and which was added.

You should be able to add this CSS to your theme for the same result:

del.diffimg img{
  outline: 2px dashed red;
  outline-offset: -3px;
}

ins.diffimg img{
  outline: 2px solid limegreen;
  outline-offset: -3px;
}

I can confirm now that on a clean install of 10.3.x using DrupalPod that toggling twig debugging (through the UI/config) does indeed result in that extra space appearing and disappearing.

So the problem, as it exists now, is not with glossify but with the twig debugging implementation, just as you suggested @tgoeg

Any line-break issues I am able to reproduce now while the twig-debugging is turned are associated with my custom template, so again, not strictly glossify's problem.

I do think that there initially factors other than twig-debugging at-play earlier on in the timeline - both because in @mjgruta'a OP we can see their devtools in the screenshot, and there's no debugging markup there, and because they to have been saying that when they deleted the empty line (which is part of Drupal's coding standards) the white-space went away, which wouldn't make sense if the cause had always been twig debugging.

Also, that same solution just over 2 months later, presumably with a different drupal version, didn't work for me in 2023.

I have the impression that some thing(s) outside of the module, in twig or in core, have been changed or improved since this issue was created, leading to some of our confusion. And it'd be worth keeping an eye on anything in core which might affect this.

If this does come back again for anybody else who ends up down the same path as me, I can say that the whitespace went away when I did any of the following:

• Deleted my custom template and fell back to the default glossify-link.html.twig
• Made sure my custom glossify-link.html.twig only contained nested elements.
• Made sure my custom glossify-link.html.twig had a wrapper element with no whitespace between the nested elements.
• Used the twig spaceless filter to achieve something similar. (I know the spaceless filter is due to be deprecated, but I wanted to see how it works. Turns out it only worked on spaces introduced by my template - the spaces introduced by twig debugging mode were untouched which makes sense. )
• As a last resort used javascript to remove the unwanted whitespace.

But if it's all working now, maybe the issue is resolved and can be closed?

Hey @robbiehobby does your re-roll in #79 include your proposed change in #78?

john cook → credited jacobupal → .

john cook → credited jacobupal → .

john cook → credited jacobupal → .

I monitored "When Designers code - UX Paper Cuts at GitLab" - Room 2

I monitored "Designing for Low Literacy: Enhancing Accessibility and Understanding" - Room 2

I monitored "70 good reasons for a code refactoring"

I monitored "Local development for environments for Drupal with DDEV"

Here is a screenshot of before the changes.

This needs to be reviewed by somebody who knows fields because now the patches have been re-rolled and turned into a merge request we can't actually confirm that the the types and identifiers present in the annotation are correct or correspond correctly with the associated field types.

I'm also removing the novice tag, as this has moved far beyond novice!

Our table is working on this!

We're working on this on our contrib table

^^ Not anymore, our bad @ultimike! We thought the 'team' meant the whole mentoring team & newbies. Good luck!

We're working on this in our contrib table.

Updated the proposed resolution with regards to providing more settings.

The changed the snippet is describing the default values not possible options.

Removed novice tag as it wouldn't be for a novice to split this into separate issues and/or meta issues.

Removing novice tag as the next steps require some specialist expertise.

I've just tried this out and while [node:body-smart-trim] alone will work, when I use it as a meta tag description [node:body-smart-trim:160] i.e. including a max-length value, it does not. The page simply loads without the appropriate meta tag present. Presumably because "[node:body-smart-trim:160]" could not be found.

So two questions:

1. Am I using the token correctly?
2. If I can't enter a character limit, what the default character limit being used?

Ok, my bad... Didn't realise this had been fixed elsewhere, just not released yet.

jacobupal → created an issue.

jacobupal → made their first commit to this issue’s fork.

I'm having this same issue, in addition to duplicates of the same message (presumably because it is somehow requested more than once, perhaps due to views or some such)... looking forward to testing this patch on alpha5

It works for me, as they say so I think it's ready for review!

I had a very similar situation, but it was removing the Media Embed icon from the toolbar which fixed the situation rather than the Media Resize filter (which didn't solve it) so the problem may be somewhere in the core Media module.

jacobupal → created an issue.

It has been included so that it can be used in the new navigation module.

Even if its discouraged, CDN does at least work, and isn't so easily broken by changes in Core, so I wouldn't write it off just yet. But if you can successfully load it from Core and that works awesome! But maybe that's getting ahead of ourselves.

On how to proceed:

• Maybe a new branch with starter-twig and folders for this sub-module included, I'd maybe call it "JS Toggletips" or similar (is there any need to say which framework is being used in the naming?)
• For now could we make it a fourth option in the tooltip style selection? Does the main module make it easy for a submodule to add a fourth option
• Include Floating-UI in the submodule, via CDN or loading it via core, whichever works the soonest
• Then we get to making it!

@alinouman Is there an easy way to check if there are pages not being counted, or is it more a case of going to each page? I'm guessing you've already compared the pages where it works to pages where it doesn't, did you find any correlation?

For us the google tag was mysteriously missing so no data was collected for two days, but I luckily caught the problem before it continued any further and I was able to add the tag again.

The comment above does make me concerned I might have pages where the data is still not being collected, it's too early to see our daily stats, but I do think our real-time stats look a bit lower than usual, we're usually 200+ per 30mins but it has seemed to have been in the 100-200 range for most of the day since I got it working. Then again, it might just be a slow day.

Just to note for those using ^10.3; #38 still seems to apply fine (10.3.1).

Just to clarify - It might just have been me who was still confused about this, but for anyone else reading: It seems 8.x-2.0-beta1 is the first release with CKEditor5 support, hurrah 🎉.

Amazing, thank you! Can't wait to try this... just need to untangle myself from my previous solution so I can test it.

Configurable-per-template is ideal for now. Not every template will need the extra markup, as it may not contain divs or similar. E.g. I'm using this module to store common tokens, and no wrapper elements are needed there. Cleaner markup is going to be preferred wherever possible which a toggle would allow.

In the long term we will want this to be addressed by this upstream issue in CKEditor5: https://github.com/ckeditor/ckeditor5/issues/6462 making these wrapper elements obsolete. In which case the fewer the instances of these wrapper elements being used now, the fewer will need to be removed later, also made possible by the use of a per-template-toggle. That is of course, if we ever get a solution to the upstream issue.

Hey @Anybody, in this instance TippyJS requires and extends PopperJS but you're right this might not be the best choice going forwards.

Both projects have since evolved into Floating UI which (at time of writing) seems to be actively maintained. Not only that but it seems that it is now part of Drupal Core as of version 10.3.0.

One day we'll be able to comfortably use HTML popover and eventually CSS Anchor Positioning and forget all about javascript libraries for such tasks!

Thanks @gaspounet

Incidentally, composer reinstall drupal/draggableviews also achieves the same thing, i.e. deleting the directory before installing the module with patches etc.

Good to hear! This might have changed through updates since I first resorted to javascript, as I am pretty sure the problem was persisting when twig-debug was disabled.

I am looking forward to trying that patch!

Pointers in the UI for how to configure your site and advice to uninstall a separate no-longer compatible module, or forced removal of another module (which I admit I don't love) feels like a fine issue to raise. But surely a separate issue; where there may be varying opinions and difficulty reaching consensus.

However the latest MR, so it seems, provides "CKEditor 5 support for Content Templates"! I really think we should consider this mission accomplished.

Improvements and changes beyond this point will always be possible and maybe even easier if we merge as is and have an established structure.

This issue is over two years old, yet we stil lack even a dev branch which supports the default CKEditor version used in the current version of Drupal. Despite a merge request sitting here which achieves just that.

It feels kinda forward for a module to uninstall another module. I don't think I have experienced that before. It's especially weird if it occurs without the users knowledge.

But if it was a notice/warning post-install and on the relevant pages and in the docs I think that would suffice no?

I recently updated to version 10.2.6 from 10.1.something and I'm getting the error but with whitespace where it should tell me what the offending attribute is, so I can't do anything to resolve it:
The following attribute(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: . I don't know what's caused this, but it made me wish extra hard that I could switch off this annoying validation... therefore very thankful for #38, cheers for that!

jacobupal → created an issue.

jacobupal → created an issue.

That'd be awesome @ericgsmith. I think that would resolve a great deal.

Having a 2.x branch makes most sense to me - as different issues can be raised with reference to that branch, so they can be tackled individually; balancing lots of different interests in single thread has been a lot!

@joshuami's suggestion looks good too!

Having an issue branch for the widget functionality (and any other contested feature) would also mean that while in development that those who do want that functionality can access it as a standalone patch.

I haven't tried it yet but it looks like this 'spaceless' option in twig could be a useful alternative that would let me do away with using javascript to do this: https://twig.symfony.com/doc/3.x/filters/spaceless.html

How's this patch working on Drupal 10?

I didn't mention that this was using Database Search.

I misunderstood, until recently, what the risks of using Database Search on a production site would be, I thought it'd just run slower than some of the alternatives, but I feel like this issue with the index cache going wonky which seems to be happening when server resource limits are hit is a bit more severe than that, even if a result of that ignorance on my part.

We're looking into Solr anyhoo.

I've had this problem too (in 10.1), which is pretty concerning on a live site... it results in search results pages just filling up with a dump of content as plain text for pages and pages... all is fixed when I clear the cache... but if I'm not around to do that it's pretty catastrophic.

I thought we fixed it by uninstalling 'Search 404' which I presumed was making more requests than some part of the system could handle. Our hosting provider also increased some of the upper limits which we presumed might have led to this.

All seemed well for two weeks or so, but now it just happened again.

It seems like the problem is somewhere on the server, but what worries me is that it fails in such a weird way... if it failed with with an understandable error that'd be better than this. I'm guessing the dumped content is just the contents of the index, and so won't contain any sensitive information but it doesn't look great. Thank you for your help!

Thank you for the reply!

I'll see how far I get with modifying the filters... "Having a separate "paste from word" style button relies on people remembering it's there and using it." -- I can totally see this, for my uses it would be the lesser of two evils though.

If it was just me using it, this would be fine - though I do generally prefer to the markup myself... However, I predict that our editor, who is not so tech savvy, and will feel the initial benefits of this module, is also going to get very confused if their formatting evaporates whilst editing a piece - I'm then going to be forever tweaking the filter rules for each edge-case that comes up. Hopefully I can can get it right. If I can't, I'll be sad to let go of this module as it is pretty awesome.

That said if I can't find another way around it, I'm sure I can make some time to look into what the implementation of an optional 'filtered paste' button would require

This'd be awesome for providing copy-able embed-code/bbcode for our staff and volunteers, but I haven't been able to get it to work.

Me too!! How do you get it to work?

I'm going to have to use rewrite condition in the .htaccess which is a shame.

jacobupal → created an issue.

Thanks @aaronpinero, I will hold tight.

I am known to move a module to my custom directory, manually compare files and apply patches line by line but this can wait!

Thank you for this! I'm having trouble with "open" being accepted in source editing, so fingers-crossed this solves my problem.

jacobupal → created an issue.

Thanks for the suggestion/compliment!

I can look into doing that in the next couple of months - my current version is a mishmash of the code above and things that only make sense in my current project.

jacobupal → changed the visibility of the branch 3398223-style-br-class to hidden.

This patch #70 → from 'CKEditor 5 support for Content Templates' ✨ CKEditor 5 support for Content Templates Needs review does a bunch of stuff to "Add widget functionality to all templates" - and this part of the patched "ckeditorTemplatesEditing.js made me wonder if we can use the schema definitions the same way, on just like a list of elements?

import { Plugin } from "ckeditor5/src/core";
import { toWidget, toWidgetEditable } from "ckeditor5/src/widget";
import { Widget } from "ckeditor5/src/widget";
import CKEditorTemplatesCommand from "./ckeditorTemplatesCommand";

/**
 * Handles the plugin functionality.
 */
export default class CKEditorTemplatesEditing extends Plugin {

  /**
   * @inheritdoc
   */
  static get requires() {
    return [Widget];
  }

  /**
   * @inheritdoc
   */
  init() {
    this._defineSchema();
    this._defineConverters();
    this.editor.commands.add(
      "ckeditorTemplates",
      new CKEditorTemplatesCommand(this.editor)
    );
  }

  /*
   * This registers the structure that will be seen by CKEditor 5 as
   *
   * The logic in _defineConverters() will determine how this is converted to
   * markup.
   */
  _defineSchema() {
    // Schemas are registered via the central `editor` object.
    const schema = this.editor.model.schema;

    schema.register("ckeditorTemplates", {
      // Behaves like a self-contained object (e.g. an image).
      isObject: true,
      // Allow in places where other blocks are allowed (e.g. directly in the root).
      allowWhere: '$block',
    });

...

There's obviously a lot more to it than I can get my head around, but if there is a way of telling CKEditor to treat all divs/sections/etc like "self-contained" objects might that be done right?

This "trapped cursor" thing is an issue with CKEditor 5 in general, as reported here in core: [GHS] Allow the cursor to leave 🐛 [GHS] Allow the cursor to leave s and other HTML elements at document start or end Postponed: needs info

here it is as a gif for brevity.

Thanks Wim, that's perfectly OK! And thanks for pushing for improvements upstream.

I just tried it on a Drupal 10.2 demo site where I added

jacobupal → created an issue.

That explains a lot! I've just been hammering composer to update from a file that doesn't exist, doh.

Actually. Ignore everything I said above. The method I wrote works exactly the same as using 'Italic' and 'Bold' buttons.

Neither button (for me) will apply to selected text, but both of them do create an empty <strong> or <em> element at the end of the summary. This is removed if you switch to source view. But if you click the 'Bold' icon and start typing right away you do get some bold text at the end of the summary.

Incidentally keyboard shortcuts Ctrl+i and Ctrl+b do both work as expected.

Span-level styles do work within the element, so until this is solved, you can use them to apply bold and italic styling to text within summaries. This is a workaround though, not a permanent solution, which only effects the appearance of the text, not the semantics.

1. add a 'bold' and 'italic' style to the Styles config in the CKEditor settings for the Text Format you are using
2. use those styles within the element
3. apply the bold and italic styling you need with CSS

Example styles:

span.force-bold|Force Bold
span.force-italic|Force Italic

Example CSS:

span.force-bold{font-weight:bold!important}
span.force-italic{font-style:italic!important}

!important; is usually not recommended, but personally I think it helps in later identifying which css rules which were hacks that need removing in future.

Thanks for the heads up. I've been postponing upgrading to 10.2 until I can adequately test stuff and this module/patch was the top of my list of concerns!

I think I worked it out.

1. I copied these lines from the webforms module to my theme's .libraries.yml file - I don't think you need the license, version and cdn stuff but I left it in:

libraries.popperjs:
  remote: https://github.com/floating-ui/floating-ui
  version: '2.11.6'
  license:
    name: MIT
    url: https://github.com/floating-ui/floating-ui/blob/v2.x/LICENSE.md
    gpl-compatible: true
  directory: popperjs
  cdn:
    /libraries/popperjs/dist/umd/: https://unpkg.com/@popperjs/core@2.11.6/dist/umd/
  js:
    /libraries/popperjs/dist/umd/popper.js: {  weight: -20  }

libraries.tippyjs:
  remote: https://github.com/atomiks/tippyjs
  version: '6.3.7'
  license:
    name: MIT
    url: https://github.com/atomiks/tippyjs/blob/master/LICENSE
    gpl-compatible: true
  directory: tippyjs
  cdn:
    /libraries/tippyjs/dist/: https://unpkg.com/tippy.js@6.3.7/dist/
  css:
    component:
      /libraries/tippyjs/dist/tippy.css: { weight: -20 }
  js:
    /libraries/tippyjs/dist/tippy.umd.js: {  weight: -20 }
  dependencies:
    - MY_THEME/libraries.popperjs

2. Then I overwrote the glossify-tooltip.html.twig template in my theme thus:

<abbr tabindex="0" {% if tip %}aria-describedby="{{ word|clean_id }}-definition"{% endif %} class="glossify-tooltip-tip despace-parent glossary-word" data-glossary-id="{{ word|clean_id }}">{{ word }}</abbr>
<span id="{{ word|clean_id }}-definition" class="glossary-definition" style="display: none;">
  <dfn><a href="{{ tipurl }}">{{ word }}</a>: </dfn>
  <span>{{ tip }}</span>
</span>

3. Then I added the following to the JS folder of my theme.

(function (Drupal) {
  'use strict';

  /**
   * Initialize tooltip element support.
   *
   * @type {Drupal~behavior}
   */
  // Check if Drupal and Drupal.behaviors are defined
  if (typeof Drupal !== 'undefined' && typeof Drupal.behaviors !== 'undefined') {
    Drupal.behaviors.glossifyTippyTooltip = {
      attach: function (context, settings) {
        // Initialize Tippy.js
        tippy('.glossary-word', {
          content: function (reference) {
            var id = reference.getAttribute('data-glossary-id');
            return document.getElementById(id + '-definition').innerHTML;
          },    
      allowHTML: true,
      interactive: true,
      trigger: 'focus click'
        });
      }
    };
  }
})(Drupal, tippy);

document.querySelectorAll('.glossary-word').forEach(function (word) {
  var tooltip = document.getElementById(word.getAttribute('data-glossary-id' + '-definition'));
  // Initialize Tippy.js
  tippy(word, {
    content: tooltip.innerHTML,
    allowHTML: true,
    interactive: true,
    trigger: 'focus click'
  });
});

4. Added my new JS as a library

glossify-tippy:
  js:
    js/glossify-tippy.js: { weight: -10 }
  dependencies:
    - MY_THEME/libraries.tippyjs

5. Added all three libraries to my theme's .info.yml file

libraries:
  - MY_THEME/libraries.popperjs
  - MY_THEME/libraries.tippyjs
  - MY_THEME/glossify-tippy

OK it's clearly a hack within a hack but this is my updated script:

(function (Drupal) {
  Drupal.behaviors.my_themeDespacer = {
    attach: function (context, settings) {

      // Define the function which removes whitespace-only nodes
      function despace(node) {
        for (var n = 0; n < node.childNodes.length; n++) {
          var child = node.childNodes[n];
          if (child.nodeType === 3 && !/\S/.test(child.nodeValue)) {
            node.removeChild(child);
            n--;
          } else if (child.nodeType === 1) {
            despace(child);
          }
        }
      }

      // Define the relevant parts of the DOM
      var targetElements = context.querySelectorAll(".despace-self, .despace-parent");

      // Call despace function based on conditions
      targetElements.forEach(function(element) {
        if (element.classList.contains('despace-parent')) {
          var parentElement = element.parentElement;
          if (parentElement) {
            despace(parentElement);
          }
        } else if (element.classList.contains('despace-self')) {
          despace(element);
        }
      });

      // Add a space between .glossify-definition definition elements and the following abbr element
      var definitionElements = context.querySelectorAll(".glossify-definition + abbr");
      definitionElements.forEach(function(element) {
        var prevElement = element.previousElementSibling;
        if (prevElement && prevElement.classList.contains('glossify-definition')) {
          var spaceNode = document.createTextNode(' ');
          prevElement.parentNode.insertBefore(spaceNode, element);
        }
      });
    }
  };
})(Drupal);

It requires that the twig template be something like this:

<abbr tabindex="0" {% if tip %}aria-describedby="{{ word|clean_id }}-definition"{% endif %} class="glossify-tooltip-tip despace-parent glossary-word" data-glossary-id="{{ word|clean_id }}">{{ word }}</abbr>
<span id="{{ word|clean_id }}-definition" class="glossify-definition" style="display: none;">
  <dfn><a href="/read/glossary#{{ word|clean_id }}">{{ word }}</a>: </dfn>
  <span>{{ tip }}</span>
</span>

Your mileage may vary!

Good find @damondt ! If so, that'd be awesome, I need to provide more than one 'horizontal line' style option in my current site build, and I'm not looking forward to getting round to that task.

jacobupal → created an issue.

jacobupal → created an issue.

jacobupal → created an issue.

OK, I got it passing... the only problem now is that the logged query will not include the hyphen so even though the test string foo bar baz works foo -bar would probably fail the test as the '-' becomes the boolean operator 'NOT' and this version strips boolean operators so the result would befoo bar. So somehow among all the array-to-string stuff the '-' needs to be brought back.

This seems to be as far as I can take it for now anyhoo.

Probably the event test should include a hyphen, and maybe even quote marks, for completeness, something like
foo -bar "fizz buzz" -"far boo"