spicywebau / craft-embedded-assets

Manage YouTube videos, Instagram photos, Twitter posts and more as first class assets in Craft CMS
MIT License
171 stars 34 forks source link

Include Inputs to capture required Craft asset field values #241

Open ctangney-tulip opened 10 months ago

ctangney-tulip commented 10 months ago

What would you like to see added to or changed in Embedded Assets, and why?

Hey! Great plug-in! We use it for pretty much every asset that we ship on our production website because it allows us to leverage our 3rd party CDN, making asset management a breeze. Including a mechanism to capture required Craft asset field values would make it way better!

Locally, we have extended your plug-in to do so, namely by including inputs on the Modal component which is rendered when a user clicks the "Embed" button --

Screenshot 2023-09-06 at 3 54 34 PM

The value from each input is then applied to the asset's field upon creation, such that required Craft fields are also required to embed an asset!

Screenshot 2023-09-06 at 3 55 04 PM

This helps to alleviate an issue where authors can easily bypass Craft CMS required fields by simply embedding an asset.

My first inclination was to re-direct users to the asset which they just created using the plug-in, but it is simple enough for them to navigate away with the back button to avoid having to enter any information.

Below, I have included a diff of the main changes --

diff --git a/environments/website/plugins/craft-embedded-assets/src/Controller.php b/environments/website/plugins/craft-embedded-assets/src/Controller.php
index e3db9b450a7c7..a7d4012aed0a6 100644
--- a/environments/website/plugins/craft-embedded-assets/src/Controller.php
+++ b/environments/website/plugins/craft-embedded-assets/src/Controller.php
@@ -52,6 +51,8 @@ public function actionSave(): Response
         $targetType = $requestService->getRequiredBodyParam('targetType');
         $targetUid = $requestService->getBodyParam('targetUid');
         $targetId = $requestService->getBodyParam('targetId');
+        $altText = $requestService->getBodyParam('altText');
+        $userTitle = $requestService->getBodyParam('title');

         if (!$targetId && !$targetUid) {
             throw new BadRequestHttpException('One of targetUid or targetId is required.');
@@ -71,6 +72,10 @@ public function actionSave(): Response
         $folder = $this->_findFolder($folderCriteria);
         $embeddedAsset = EmbeddedAssets::$plugin->methods->requestUrl($url);
         $asset = EmbeddedAssets::$plugin->methods->createAsset($embeddedAsset, $folder);
+        $asset->alt = $altText;
+        if ($userTitle) {
+            $asset->title = $userTitle;
+        }
         $result = Craft::$app->getElements()->saveElement($asset);

         if (!$result) {
diff --git a/environments/website/plugins/craft-embedded-assets/src/assets/main/src/scripts/Form.ts b/environments/website/plugins/craft-embedded-assets/src/assets/main/src/scripts/Form.ts
index f0a0762c17515..9248a71d41df2 100644
--- a/environments/website/plugins/craft-embedded-assets/src/assets/main/src/scripts/Form.ts
+++ b/environments/website/plugins/craft-embedded-assets/src/assets/main/src/scripts/Form.ts
@@ -63,17 +63,21 @@ export default class Form extends Emitter {
       }
     })

-    this.preview.on('resize', (e: PreviewResizeEvent) => this.$body?.css('height', `${e.height}px`))
+    this.preview.on('resize', (e: PreviewResizeEvent) => this.$body?.css('height', `${e.height + 50}px`))

     this.$element.on('submit', (e) => {
       e.preventDefault()

-      const isSameUrl = this._url === this.$input?.val()
+      if ((document.querySelector('.embedded-assets_alt-text > input') as HTMLInputElement)?.value.length === 0) {
+        document.querySelector('.embedded-assets_alt-text > input')?.classList.add('error')
+      } else {
+        const isSameUrl = this._url === this.$input?.val()

-      if (this._state === 'idle' || (this._state !== 'saving' && !isSameUrl)) {
-        this.request()
-      } else if (this._state === 'requested' && isSameUrl) {
-        this.save()
+        if (this._state === 'idle' || (this._state !== 'saving' && !isSameUrl)) {
+          this.request()
+        } else if (this._state === 'requested' && isSameUrl) {
+          this.save()
+        }
       }
     })

@@ -135,32 +139,49 @@ export default class Form extends Emitter {
   }

   public save (url = this.$input?.val() as string, actionTarget = this._getActionTarget()): void {
-    const data = {
-      ...actionTarget,
-      url,
-      assetId: this._replaceAssetId
+    const titleValue = (document.querySelector('.embedded-assets_title > input') as HTMLInputElement).value
+    const altTextValue = (document.querySelector('.embedded-assets_alt-text > input') as HTMLInputElement).value
+
+    if (titleValue.length === 0) {
+      document.querySelector('.embedded-assets_title')?.classList.add('required')
     }

-    Craft.queue.push(async () => await new Promise((resolve, reject) => {
-      Craft.sendActionRequest('POST', `embeddedassets/actions/${(this._replace ? 'replace' : 'save')}`, { data })
-        .then((response: PreviewResponse) => {
-          if (this._state === 'saving' && typeof response.data?.success !== 'undefined') {
-            this.clear()
-            this.trigger('save', response.data.payload)
-            resolve(undefined)
-          } else {
-            if (typeof response.data?.error !== 'undefined') {
-              Craft.cp.displayError(response.data.error)
-            }
+    if (altTextValue.length === 0) {
+      document.querySelector('.embedded-assets_alt-text')?.classList.add('required')
+    }

-            this.setState('requested')
-            reject(new Error(response.data?.error))
-          }
-        })
-        .catch(reject)
-    }))
+    if (titleValue.length > 0 && altTextValue.length > 0) {
+      const data = {
+        ...actionTarget,
+        url,
+        altText: altTextValue,
+        title: titleValue,
+        assetId: this._replaceAssetId
+      }
+
+      Craft.queue.push(async () => await new Promise((resolve, reject) => {
+        Craft.sendActionRequest('POST', `embeddedassets/actions/${(this._replace ? 'replace' : 'save')}`, { data })
+          .then((response: PreviewResponse) => {
+            if (this._state === 'saving' && typeof response.data?.success !== 'undefined') {
+              this.clear()
+              this.trigger('save', response.data.payload)
+              resolve(undefined)
+            } else {
+              if (typeof response.data?.error !== 'undefined') {
+                Craft.cp.displayError(response.data.error)
+              }
+
+              this.setState('requested')
+              reject(new Error(response.data?.error))
+            }
+          })
+          .catch(reject)
+      }))

-    this.setState('saving')
+      this.setState('saving')
+    }
   }

   public setState (state: string): void {
diff --git a/environments/website/plugins/craft-embedded-assets/src/assets/main/src/scripts/Modal.ts b/environments/website/plugins/craft-embedded-assets/src/assets/main/src/scripts/Modal.ts
index 1c70b3b793906..d66fbb9a8adea 100644
--- a/environments/website/plugins/craft-embedded-assets/src/assets/main/src/scripts/Modal.ts
+++ b/environments/website/plugins/craft-embedded-assets/src/assets/main/src/scripts/Modal.ts
@@ -34,6 +34,14 @@ export default class Modal extends Emitter {

     this.$footer = $(`
       <div class="hud-footer embedded-assets_hud_footer">
+        <div class="embedded-assets_title">
+          <label for="embedded-assets_title-input">Title required</label>
+          <input id="embedded-assets_title-input" type="text=" placeholder="Enter title" />
+        </div>
+        <div class="embedded-assets_alt-text">
+          <label for="embedded-assets_alt-text-input">Alt text required</label>
+          <input id="embedded-assets_alt-text-input" type="text=" placeholder="Enter alt text" />
+        </div>
         <div class="buttons right">
           <div id="${cancelId}" class="btn">${cancelLabel}</div>
           <div id="${saveId}" class="btn submit">${saveLabel}</div>
@@ -42,6 +50,18 @@ export default class Modal extends Emitter {
       </div>
     `)

+    this.$footer.find('.embedded-assets_title input').on('blur', (e: any) => {
+      e.target.value.length > 0
+        ? e.target.parentElement.classList.remove('required')
+        : e.target.parentElement.classList.add('required')
+    })
+
+    this.$footer.find('.embedded-assets_alt-text input').on('blur', (e: any) => {
+      e.target.value.length > 0
+        ? e.target.parentElement.classList.remove('required')
+        : e.target.parentElement.classList.add('required')
+    })
+
     this.$cancel = this.$footer.find(`#${cancelId}`)
     this.$save = this.$footer.find(`#${saveId}`)
     this.$spinner = this.$footer.find(`#${spinnerId}`) 
 //

In general things are pretty specific for our implementation, but it should be a good head start toward implementing something more general.

I could envision a section within the settings for the plugin where required asset fields are captured, such that a mapping can be generated programmatically between:

The addition of the above, coupled with code changes to make our implementation more abstract, would be a lovely add to this plugin! Sadly, I cannot give my undivided attention to this feature, which is what I feel it would deserve.

Cheers!

ttempleton commented 10 months ago

This is a fair point, and something I hadn't considered. It would be worth us looking at replacing the existing preview modal with a slideout element editor to achieve this.

ctangney-tulip commented 10 months ago

More than happy to support as my capacity allows for it!