Account created on 18 October 2013, about 11 years ago
#

Merge Requests

Recent comments

πŸ‡ΊπŸ‡ΈUnited States nessthehero

That module is too experimental for our use case. I also feel like what I'm trying to do should work out of principle. It doesn't seem that far out there to just be able to override things in PHP.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

It might be necessary to add a new hook that allows disabling Metatag entirely for a route.. or maybe extend Metatag Custom Routes to allow this as an option?

Yes, this sounds promising. Happy to help in any way I can.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Just to give a sampling of what I've tried so far:

function cmu_expertfile_metatag_route_entity() {
  if (strpos(\Drupal::routeMatch()->getRouteName(), 'cmu_expertfile') !== FALSE) {
    return [];
  }
}

function cmu_expertfile_metatags_alter(array &$metatags, array $context) {
  $metatags['og:description'] = '-2- Beep boop website stuff';
}

function cmu_expertfile_metatags_attachments_alter(array &$attachments) {
  if (isset($attachments['#attached']['html_head'])) {
    foreach ($attachments['#attached']['html_head'] as $key => &$attachment) {
      if ($attachment[1] == 'og:description') {
        $attachments['#attached']['html_head'][$key][0]['#attributes']['content'] = 'Beep boop website stuff';
      }
    }
  }
}

function cmu_expertfile_page_attachments_alter(array &$page) {
  if (!empty($page['#attached']['html_head'])) {
    foreach ($page['#attached']['html_head'] as &$attachment) {
      if ($attachment[1] == 'og:description') {
        $attachment[0]['#attributes']['content'] = 'Beep boop website stuff';

      }
    }
  }
}

I'm just trying to get literally anything to show up in the Global og:description meta tag from my custom module, and I can't override it this way.

I would expect that adding a metatag to html_head would at some point reduce any duplicates, deferring to the most recent attachment. If the metatag module is set up to add Global metatags last, I would also then expect that module to detect existing items first.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Unfortunately I'm not seeing any results from this. Returning null doesn't change any of the global metatags that are pulled into the page.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

@damienmckenna Do you have more info on that hook? I can't find it documented anywhere and I can't pause inside it with XDebug.

Just updated my local metatag to 2.1.0 as well.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Updated to beta2.

Seems like it's working better. I was having some issues in one environment but I think there's other stuff in the project interfering with it, so I made the change in a more barebones Drupal install. The only issue I saw was that clicking "Original" didn't seem to clear the resize off the image. It was still resized after I saved, and still the same size when I went back in to edit.

I can't guarantee there isn't something still interfering with what I have so I would suggest if someone else is able to test and reproduce these problems.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

I'm probably not. Let me check that and I'll report back later today.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Tried this out and it works. The only problem I noticed was that it undoes the resizing when you edit the node and are looking at the WYSIWYG in the edit form. It doesn't remove the resized value (e.g. if you save, you still have the last value you resized to) but it may be jarring to a non-technical content editor.

Testing steps (after placing new JS file)
1. Create a node that has a WYSIWYG.
2. Place a media item (We'll say it's 750px wide) in the WYSIWYG.
3. Save
4. Inspect and see that image is not resized.
5. Edit and resize image to 525px.
6. Save
7. Inspect and see that image is wrapped in a div with `width:525px` in a style attribute
8. Edit the node. You should notice that the image in the WYSIWYG looks like it has reverted back to its original size. Clicking on the image will have it say "Original" on the resize style dropdown.

If I inspect source, I see `undefined="525px"` on the drupal-media tag.

If I try to resize the image after this, I see LOTS of console errors.

ckeditor5-dll.js?v=41.3.1:5 Uncaught CKEditorError: cannot-change-view-tree
Read more: https://ckeditor.com/docs/ckeditor5/latest/support/error-codes.html#error-cannot-change-view-tree
    at St.change (ckeditor5-dll.js?v=41.3.1:5:258636)
    at ie.updateSize (ckeditor5-dll.js?v=41.3.1:5:723987)
    at ie.<anonymous> (ckeditor5-dll.js?v=41.3.1:5:666767)
    at ie.fire (ckeditor5-dll.js?v=41.3.1:5:663135)
    at <computed> [as updateSize] (ckeditor5-dll.js?v=41.3.1:5:666819)
    at ce._mouseMoveListener (ckeditor5-dll.js?v=41.3.1:5:729770)
    at ue.fire (ckeditor5-dll.js?v=41.3.1:5:663135)
    at HTMLDocument.t (ckeditor5-dll.js?v=41.3.1:5:673313)
πŸ‡ΊπŸ‡ΈUnited States nessthehero

#12 Yes this "works" but is unacceptable as a solution. You can't expect standard standard users to perform a trick to fix their content.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Blocking issue for us. It prevents the resize value from updating on subsequent saves. Would appreciate some clarity in why this filter is needed. We cannot turn it on for customer-provided reasons.

Annoying that this is the only module that lets you do this functionality.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

I also encountered this issue but decided to mitigate it by removing the "Save and insert" button action and renaming the "Save and select" action to "Save Media".

I used a custom module with a form alter hook.

function HOOK_form_alter(&$form, \Drupal\Core\Form\FormStateInterface $form_state, $form_id) {

  if ($form_id === 'media_library_add_form_upload') {

    if (isset($_GET['media_library_opener_id']) && $_GET['media_library_opener_id'] === 'media_library.opener.bulk_upload') {
      if (isset($form['actions'])) {
        $form['actions']['save_select']['#value'] = new TranslatableMarkup('Save Media');
        unset($form['actions']['save_insert']);
      }
    }

  }

}

This only affects the bulk upload form from the media overview screen, and not any individual upload field found on a node edit form. Hopefully this helps someone.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

I've noticed that the itok key in the request on my localhost is different than if I went directly to the source environment and found the same image.

Not sure if related. If I update the local proxy redirect to use the same itok as the source environment, the image request works.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Just chiming in that the patch in #18 works for 2.x on a Drupal 10.1 install. This is exactly what I needed!

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Was able to spin up an instance using the MR and write a custom module that ties into this event. Seems to work great!

((Drupal, $) => {

  Drupal.behaviors.testEditorAttachment = {
    attach() {

      console.log('behavior attached');

      $(document).on('editor:attached', function (event, editor) {
        if (typeof editor !== "undefined") {
          console.log('Event works!', event, editor);

          editor.setData('<p>Hello world!</p>');
        }
      });

    }
  };

})(Drupal, jQuery);

It's wishful thinking, but it'd be nice if this used native web Event Listeners rather than jQuery events, to reduce another need for jQuery in the large scheme of things, but it works as expected.

I won't mark as RTBC since this needs tests, but this is definitely something I could use in my current projects.

Would it be possible to roll this for 10.1? Is there a reason it must wait til 11?

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Noticed two things while digging into this:

1. The role is never passed into getRecords. It's always either '' or 'anonymous'.
2. If I modify getAccessContentStatus to pass the user's role, it then looks for records that match the UID _and_ Role. I'm not setting the specific user id for my restrictions so this is definitely odd.

I pushed up a change that improves the check for records using roles, and if the user is logged in and hits a page that is set to redirect to the login (and their role is restricted), they get a 403.

Also adding in a new patch.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

I am not able to reproduce that behavior.

Here are my testing steps:

1. Create a new node that is restricted to "Visitor", and select the option "User must log in".
2. Visit the node in an incognito window. I am redirected to the login screen.
3. Do not log in. Instead, modify the url to be the url of the restricted page. Hit enter to navigate.
4. I am redirected back to the login screen again.

If you are logged in already, then it won't redirect you to the login screen or destroy your session. Perhaps what needs done is to show a 403 if you are logged in and try to hit the restricted page.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

I am attaching a patch as well in case anyone wants to use this immediately in a project.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

+1 to #24. If the plugin is free to use, it should be separated from the rest of the "pay to use" plugins, as it is confusing to be required to pay for all except one of the included plugins under a nomenclature of "premium".

I've tried out the fullscreen feature on a local site and it's pretty much exactly what our users want, so it would be nice if it was standalone.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

I can only test the "DAM - Image" content type since I don't think we have any other kinds of content in our DAM (and I don't have the ability to create/add assets to it), so I can mark this as RTBC for that aspect!

Excellent work! It should be documented somewhere that in order for the thumbnail to appear, you have to move the "Asset Reference" field out from the disabled fields on the Form Display settings for the DAM - Image media type.

Thanks!

πŸ‡ΊπŸ‡ΈUnited States nessthehero

We recently updated to Drupal 10.1 but aren't currently using the acquia_dam module (due to user need), but the versions I had when this issue was created were:

Drupal Core 9.5.9
AdvAgg 6.0.0-alpha1
Acquia DAM 1.0.11

We also haven't seen any issues related to AdvAgg on 10.1, at least not yet.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

The problem seems to be related to the Devel module. I needed to re-export some configuration and commit that to our project repo, so that when the configuration is imported, the configuration for Devel is correct.

Not sure why this caused an error but it's definitely not related to this module. Sorry about that.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

LGTM! Module works on Drupal 10.1.2 via DrupalPod, and changes in the MR seem simple enough to me.

Also tested in 9.5.9 with Upgrade Status, and it shows up green!

I'm still a bit new to contributing on d.o so I don't know if I need to mark this as RTBC or someone else can do that? Either way, I think this is great!

πŸ‡ΊπŸ‡ΈUnited States nessthehero

@flyke Patch #57 replaces patch #43 for dropzone, it is not a patch for the media_bulk_upload.

"drupal/media_bulk_upload": {
  "3023825-53: Allow alt text to be modified on upload": "https://www.drupal.org/files/issues/2023-06-27/media_bulk_upload-3023825-53.patch"
},
"drupal/dropzonejs": {
  "3023825-57: Better UX for bulk upload": "https://www.drupal.org/files/issues/2023-08-16/dropzonejs-3023825-57.patch"
}

This is a great improvement.

Something else I would like to see is the ability to choose if I am sent back to the media overview listing rather than back to the bulk upload form, but it's a lower priority thing.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Attached a patch that adds a semicolon after the template string in acquia-dam-categories.js.

This seems to be the easiest way to fix it, though I understand if yinz have coding standards and omit semicolons intentionally. I have this patch applied locally for my project and it fixes the issue.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Is this task under consideration by the maintainers? This module shows up on Upgrade Status under "Collaborate with maintainers" as a module that needs support.

I don't see any other ticket tracking active progress towards Drupal 10 support. Is it planned? Is this module still actively maintained?

πŸ‡ΊπŸ‡ΈUnited States nessthehero

I have noticed as well if you are typing in an Alt field, and you hit enter, it tries to submit all the images, and if the alt text wasn't saved then it would throw an error, which clears out the images completely. I feel like this is a critical bug.

Perhaps the functionality should:
1. Auto save when a user types and either pauses or blurs the field
2. Hitting enter saves the metadata if the focus is on the alt or title fields, and does not submit the total form
3. Submitting the form saves each metadata if it has values, and does not submit if any required fields do not have data

πŸ‡ΊπŸ‡ΈUnited States nessthehero

I agree with the most recent comment that the "Save metadata" button should be removed in favor of having any changes to the fields be saved when the "Submit" button is pressed. It is a more convenient method for a standard content editor who may not understand they need to click each button to save the data any time they make a change.

The alt text being required should also prevent the images from being saved but not remove the images from the upload tray. Hitting submit should trigger inline validation on those fields. The label for "Alt" should also have some kind of indicator that the field is required, like an asterisk or color treatment.

The look of the added Alt and Title fields is very ugly and the markup that constitutes that form is very plain and hard to style. It needs some CSS attention to make it look more presentable. I recommend wrapping those fields in an additional container (fieldset?).

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Just another thought on the matter:

I think it would be a better user experience if the "Select media source" dropdown defaulted to the "Media Types" selection, allowing the user to always be able to select internal site assets, and only prompted authentication when the "DAM" source is selected.

It seems like the module is coded to show the authentication prompt the first time a user ever opens a media modal window.

It would selfishly solve my issue I'm still having, but I think it would be a better experience overall.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Gotcha.

I'll try and see what we can do about it, but regardless, this ticket can be considered resolved.

Thanks for your help!!

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Okay, I believe I have now set it up correctly. Thanks for the advice!

I do have a follow up question. When the authentication method is set to a request token, why does the site still need authenticated to a single user's account?

Basically, we're getting hung up with Behat tests, when they try to attach a Media image to a field in a few tests they run into the modal message that asks to authenticate the site. I'm hoping there is a way to avoid this completely. Is that possible?

πŸ‡ΊπŸ‡ΈUnited States nessthehero

@mlgaman Thanks for the reply!

I'm interested in using the configuration overrides to store the values. We want to use the refresh token flow but store the keys in .env files.

Does that make sense?

I'm trying this right now in a settings.php file:

  $config['acquia_dam.settings']['auth_type'] = 'refresh_token';
  $config['acquia_dam.settings']['client_id'] = getenv('WIDEN_CLIENT_ID');
  $config['acquia_dam.settings']['client_secret'] = getenv('WIDEN_CLIENT_SECRET');

But it does not appear to work as I would expect. The "Refresh Token" radio button is not selected and the two OAuth fields are empty.

I also tried commenting out the "getenv" functions and pasting in the key directly, but that did not work either.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

I have attached a patch that adds a global option to the settings form for Access Unpublished to clear tokens when a node is published.

Any tokens created while a node is unpublished will be deleted when that node is finally published.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

I may need to rework this briefly. Sending it back to needs work. I'll have another patch soon once I have some more testing.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Good afternoon!

Here is some feedback I had:

Regarding the "results" page: (/admin/modules/browse)

1. I wonder if it would be valuable to have either a category or a filter for "Sponsored" modules. Ideally, modules that are sponsored, funded, or wholly driven by one or more organizations. Perhaps even some kind of iconography on each box that identifies a module is sponsored (not necessarily the logo of a company/s, but a more generic "is sponsored" identifier.)

I feel like it is attractive to a site owner to see a module has the backing of a large organization. It can give a clearer picture of how stable/supported a module is. If a site owner needed approval to install a module, a sponsored module may be an easier sell to a stakeholder.

2. I feel like it would be a simple add to have a reference to the latest version number available of a module somewhere on the box of each result. This may be useful if the "developer recommended" version of a module is a -dev or -alpha or -beta version, which some site owners may wish to avoid.

Regarding the "module details" page: (e.g. /admin/modules/browse/metatag)

1. Sponsoring individuals/organizations would be very useful on this page. They are currently mentioned on drupal.org. Perhaps in the sidebar of this page?

2. Once the module is installed, perhaps this page could have links to the Issue Queue for the module. Possibly even a one-click "Create an issue" button to allow them to immediately jump to creating an issue.

This could have even further potential to pre-populate some of the issue description with information about their site, such as modules installed, PHP version, etc.

3. Direct links to documentation sources would be useful here.

πŸ‡ΊπŸ‡ΈUnited States nessthehero

Also interested in this. If not allowing resizing by clicking and dragging, an editor should be able to provide a width and height to the media item.

Production build 0.71.5 2024