Skip to content

Conversation

@bembelimen
Copy link
Contributor

@bembelimen bembelimen commented Nov 29, 2020

Summary of Changes

When creating a 3rd party filesystem plugin and selecting an image in a media file selection (e.g. in the article form view), the image will not be displayed, because the variable is wrong.

Testing Instructions

Actual result BEFORE applying this Pull Request

  • Image not selected/displayed

Expected result AFTER applying this Pull Request

  • Image selected/displayed

@dgrammatiko perhaps you can check?

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Nov 29, 2020
@dgrammatiko
Copy link
Contributor

Is thump_path always the same as the url? If so then remove it in the Media manager Api, but I think that thump is for a thumbnail provided by some cloud storage providers (s3, dropbox)

@bembelimen
Copy link
Contributor Author

Is thump_path always the same as the url? If so then remove it in the Media manager Api, but I think that thump is for a thumbnail provided by some cloud storage providers (s3, dropbox)

No, but that is not the fix here. Thumbpath is for non local adapters. This fix is the call, when someone either insert an image into the editor (via the image plugin) or uses the image media field (like the intro image). In this case only the "url" is relevant. I search the whole Joomla for Joomla.selectedMediaFile.thumb which is never used, I assume, that is is a leftover or a typo?

So the value filled in has to be Joomla.selectedMediaFile.url to functional.

@dgrammatiko
Copy link
Contributor

dgrammatiko commented Nov 29, 2020

So the value filled in has to be Joomla.selectedMediaFile.url to functional.

Yes, you're right. FWIW the thumb part shouldn't be there but rather in

updatePreview() {
if (['true', 'static'].indexOf(this.preview) === -1 || this.preview === 'false' || !this.previewElement) {
return;
}
// Reset preview
if (this.preview) {
const { value } = this.inputElement;
if (!value) {
this.previewElement.innerHTML = '<span class="field-media-preview-icon"></span>';
} else {
this.previewElement.innerHTML = '';
const imgPreview = new Image();
const mediaType = {
image() {
imgPreview.src = /http/.test(value) ? value : Joomla.getOptions('system.paths').rootFull + value;
imgPreview.setAttribute('alt', '');
},
};
mediaType[this.type]();
this.previewElement.style.width = this.previewWidth;
this.previewElement.appendChild(imgPreview);
}
}
}
something like:

updatePreview() {
  if (['true', 'static'].indexOf(this.preview) === -1 || this.preview === 'false' || !this.previewElement) {
    return;
  }

  // Reset preview
  if (this.preview) {
    const { value } = this.inputElement;

    if (!value) {
      this.previewElement.innerHTML = '<span class="field-media-preview-icon"></span>';
    } else {
      let thumb = Joomla.selectedMediaFile.url;
      this.previewElement.innerHTML = '';
      const imgPreview = new Image();

      if (Joomla.selectedMediaFile.thumb) {
        thumb = Joomla.selectedMediaFile.thumb;
      }

      const mediaType = {
        image() {
          imgPreview.src = /http/.test(thumb) ? thumb : Joomla.getOptions('system.paths').rootFull + thumb;
          imgPreview.setAttribute('alt', '');
        },
      };

      mediaType[this.type]();

      this.previewElement.style.width = this.previewWidth;
      this.previewElement.appendChild(imgPreview);
    }
  }
}

@Quy Quy added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Dec 28, 2020
@brianteeman
Copy link
Contributor

Is this on hold or should it still be tested?

@ghost
Copy link

ghost commented Jun 9, 2021

Can a maintainer please set the label "Conflicting Files"?

@bembelimen bembelimen marked this pull request as draft January 25, 2022 05:09
@chmst chmst changed the base branch from 4.0-dev to 4.1-dev January 31, 2022 15:27
@dgrammatiko
Copy link
Contributor

This should be already fixed by now

@HLeithner HLeithner changed the base branch from 4.1-dev to 4.2-dev June 27, 2022 13:08
@HLeithner
Copy link
Member

This pull request has automatically rebased to 4.2-dev.

@rdeutz
Copy link
Contributor

rdeutz commented Oct 21, 2022

What should we do with this PR @bembelimen @dgrammatiko

@rdeutz rdeutz added the Maintainers Checked Used if the PR is conceptional useful label Oct 21, 2022
@dgrammatiko
Copy link
Contributor

If #38805 is merged then also local adapters would have a thumb so this PR would break the other one.

@bembelimen bembelimen closed this Oct 21, 2022
@bembelimen bembelimen deleted the 4.0/media-insert-image branch March 15, 2024 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Conflicting Files Maintainers Checked Used if the PR is conceptional useful NPM Resource Changed This Pull Request can't be tested by Patchtester Updates Requested Indicates that this pull request needs an update from the author and should not be tested.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants