Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Users from drag/drop and copy/paste images into the text editor #1270

Closed
AbhishekPAnil opened this issue Aug 5, 2024 · 24 comments · Fixed by #1275
Closed

Prevent Users from drag/drop and copy/paste images into the text editor #1270

AbhishekPAnil opened this issue Aug 5, 2024 · 24 comments · Fixed by #1275
Assignees
Labels
bug Something isn't working enhancement New feature or request To be deployed Tout Culture

Comments

@AbhishekPAnil
Copy link
Contributor

AbhishekPAnil commented Aug 5, 2024

Currently, users can copy/paste and drag/drop images directly into the text editor. This behavior can lead to slowing down of the listing api's , since the images are not passed through our image server. To maintain a consistent user experience and prevent potential problems, we need to disable the ability to copy/paste and drag/drop images into the text editor.
Ref slack message : https://culturecreates.slack.com/archives/C02B18SN3FU/p1722702723875049

Expected Behavior

  • Users should not be able to paste images into the text editor.
  • Users should not be able to drag and drop images into the text editor.

Steps to Reproduce

  1. Open the any form(Event, People, Place, Organization).
  2. Copy an image from any source (e.g., web page, local file).
  3. Paste the image into the text editor.
  4. Drag an image from any source and drop it into the text editor.
  5. Save the entity. The image is embedded into the text editor.

Proposed Solution

  • Disable the copy/pasting using the quill format.
  • Disable the drag/drop using the onDrop function.

Also ,ensure that other types of content (e.g., text, links) can still be copied/pasted and dragged/dropped into the text editor.

@AbhishekPAnil AbhishekPAnil self-assigned this Aug 5, 2024
@AbhishekPAnil AbhishekPAnil added the enhancement New feature or request label Aug 5, 2024
@troughc troughc assigned troughc and unassigned AbhishekPAnil Aug 5, 2024
@troughc troughc added the blocked label Aug 5, 2024
@troughc
Copy link
Contributor

troughc commented Aug 5, 2024

Caitlin needs to discuss this change with customer.

@troughc
Copy link
Contributor

troughc commented Aug 5, 2024

@AbhishekPAnil can users add (embed) video content to descriptions? Or is that not allowed as well?

@AbhishekPAnil
Copy link
Contributor Author

@AbhishekPAnil can users add (embed) video content to descriptions?

Need to test if they can embedd videos to description.
We already have an additional field to embed the url.Right?

@troughc
Copy link
Contributor

troughc commented Aug 5, 2024

Yes we do already have a field for that. I am asking just to know what the text field currently supports. Not suggesting people should add video to descriptions. :)
If memory serves me right, they CANNOT add videos.

@troughc
Copy link
Contributor

troughc commented Aug 5, 2024

I was able to embed in this test event (draft), but I don't know what the impact is or if it works.
https://staging.cms.footlight.io/dashboard/6308ef4a7f771f00431d939a/events/66b0e05ace7a2500a39f9c5c

@kmdvs
Copy link
Collaborator

kmdvs commented Aug 5, 2024

A mitigation possibility: Is there a way to set up a MB limit, to accommodate any combination of number of files / filesize?

ChatGPT
Yes, you can set up a total file size limit (in MB) that accommodates any combination of the number of files and their individual sizes. This approach allows you to enforce a maximum total upload size, regardless of how many files are selected or their individual sizes.

Here's an example of how you can implement this in a React component:

Example React Component with Total File Size Limit


import React, { useState } from 'react';

function ImageUpload() {
  const [files, setFiles] = useState([]);
  const [totalSize, setTotalSize] = useState(0); // Total size in MB
  const maxTotalSize = 10; // Maximum total size in MB

  const handleDragOver = (event) => {
    event.preventDefault();
  };

  const handleDrop = (event) => {
    event.preventDefault();
    const droppedFiles = Array.from(event.dataTransfer.files);
    let newTotalSize = totalSize;

    // Calculate the total size of new files
    droppedFiles.forEach(file => {
      newTotalSize += file.size / 1024 / 1024; // Convert size to MB
    });

    // Check if the total size exceeds the maximum allowed size
    if (newTotalSize > maxTotalSize) {
      alert(`Total file size cannot exceed ${maxTotalSize} MB. Please select smaller files.`);
      return;
    }

    // Set the new files and update the total size
    setFiles((prevFiles) => [...prevFiles, ...droppedFiles]);
    setTotalSize(newTotalSize);
  };

  return (
    <div>
      <div
        onDragOver={handleDragOver}
        onDrop={handleDrop}
        style={{
          width: '300px',
          height: '200px',
          border: '2px dashed #ccc',
          display: 'flex',
          alignItems: 'center',
          justifyContent: 'center',
          margin: '50px auto',
        }}
      >
        Drop images here
      </div>
      <div>
        <p>Total size: {totalSize.toFixed(2)} MB</p>
        {files.map((file, index) => (
          <p key={index}>
            File {index + 1}: {file.name}, Size: {(file.size / 1024 / 1024).toFixed(2)} MB
          </p>
        ))}
      </div>
    </div>
  );
}

export default ImageUpload;

Explanation
State Management:

files: An array that holds the dropped files.
totalSize: A state variable that tracks the total size of all files in MB.
maxTotalSize: A constant representing the maximum allowed total file size in MB.
handleDragOver Function:

Prevents the default behavior to allow the drop event.
handleDrop Function:

Prevent Default Behavior: Prevents the default drop action.
Convert Files: Converts the FileList object to an array using Array.from.
Calculate Total Size: Iterates over the dropped files and calculates their total size in MB, adding to the existing totalSize.
Check Total Size: Compares the new total size with maxTotalSize. If it exceeds the limit, an alert is shown, and the function returns, preventing the addition of new files.
Update State: If the total size is within the limit, the new files are added to the existing state, and the totalSize state is updated.
Rendering:

The drop zone is defined with onDragOver and onDrop event handlers.
The total size of the uploaded files and details of each file are displayed.
Additional Features
File Removal: You can add functionality to remove individual files from the list, which would also update the totalSize state accordingly.
File Validation: Additional checks, such as file type validation or individual file size limits, can be incorporated to provide a more comprehensive validation.
This implementation ensures that the total size of all files does not exceed a specified limit, allowing for any combination of file numbers and sizes within that limit.

@troughc
Copy link
Contributor

troughc commented Aug 6, 2024

@troughc
Copy link
Contributor

troughc commented Aug 6, 2024

Note to @AbhishekPAnil: For now, do not stop users from embedding video from youtube into the text field. Please check what happens if you drag/copy-paste video into the description - if it's a link, it's okay; if its huge, that doesn't work

@AbhishekPAnil
Copy link
Contributor Author

cc @AbhishekPAnil
As per discussion ,

  1. disable drag/drop of image/video into the editor.
  2. Add a button to upload image/video.
  3. Check if its possible upload the image directly from the editor to our image server.

@AbhishekPAnil AbhishekPAnil self-assigned this Aug 7, 2024
@troughc troughc removed the blocked label Aug 7, 2024
@AbhishekPAnil
Copy link
Contributor Author

AbhishekPAnil commented Aug 7, 2024

The only issue faced now is ,while uploading the image from the local system into the editor,the image is saved in the form of an image tag with a base64 src data

@AbhishekPAnil
Copy link
Contributor Author

The only issue faced now is ,while uploading the image from the local system into the editor,the image is saved in the form of an image tag with a base64 src data

This will be solved by uploading the image to the s3 and then using the url from the api response to update the src inside the image tag.

@sahalali
Copy link
Contributor

@AbhishekPAnil
I've added support to return the original image URL in the image upload API response. This is now live in staging.

@AbhishekPAnil AbhishekPAnil linked a pull request Aug 12, 2024 that will close this issue
@AbhishekPAnil AbhishekPAnil removed their assignment Aug 12, 2024
@troughc
Copy link
Contributor

troughc commented Aug 12, 2024

For my test: To check the image data, I will inspect the image in the read-only page. Otherwise, I can check with Abhishek

@troughc
Copy link
Contributor

troughc commented Aug 12, 2024

@AbhishekPAnil

1. Image tests:

https://staging.cms.footlight.io/dashboard/6308ef4a7f771f00431d939a/events/66ba19674be0ab00a039defa
I was able to copy paste an image from google. Then, when I inspected it, I see this - it says base64 in the text when I hover over it? Is that okay?

Screenshot 2024-08-12 at 10 25 51 AM Screenshot 2024-08-12 at 10 26 02 AM

Please check these three events. https://pr-1275.dla06mbe0wyl7.amplifyapp.com/dashboard/6308ef4a7f771f00431d939a/events?page=1&order=asc&sortBy=upcomingEventDate&query=caitlin
(Search on Caitlin and you will see three test events)

In test event 3, some of the images also look like they are base64. Even ones that I uploaded from my desktop using the upload button. Test 2 seems okay, but please check

2. Video link test:

https://pr-1275.dla06mbe0wyl7.amplifyapp.com/dashboard/6308ef4a7f771f00431d939a/events/66ba327c4be0ab00a039e5b3

I can add the video link but the link is not completely visible:
Screenshot 2024-08-12 at 12 03 30 PM

3. When I publish, too many popups

I get to success messages instead of one. You can reproduce it by saving any of my test events as draft and then republish.
Screenshot 2024-08-12 at 11 57 41 AM

@troughc troughc assigned AbhishekPAnil and unassigned troughc Aug 12, 2024
@troughc troughc added the bug Something isn't working label Aug 12, 2024
@AbhishekPAnil
Copy link
Contributor Author

@AbhishekPAnil

1. Image tests:

https://staging.cms.footlight.io/dashboard/6308ef4a7f771f00431d939a/events/66ba19674be0ab00a039defa
I was able to copy paste an image from google. Then, when I inspected it, I see this - it says base64 in the text when I hover over it? Is that okay?

Screenshot 2024-08-12 at 10 25 51 AM Screenshot 2024-08-12 at 10 26 02 AM

Please check these three events. https://pr-1275.dla06mbe0wyl7.amplifyapp.com/dashboard/6308ef4a7f771f00431d939a/events?page=1&order=asc&sortBy=upcomingEventDate&query=caitlin
(Search on Caitlin and you will see three test events)

In test event 3, some of the images also look like they are base64. Even ones that I uploaded from my desktop using the upload button. Test 2 seems okay, but please check

2. Video link test:

https://pr-1275.dla06mbe0wyl7.amplifyapp.com/dashboard/6308ef4a7f771f00431d939a/events/66ba327c4be0ab00a039e5b3

I can add the video link but the link is not completely visible:
Screenshot 2024-08-12 at 12 03 30 PM

3. When I publish, too many popups

I get to success messages instead of one. You can reproduce it by saving any of my test events as draft and then republish.
Screenshot 2024-08-12 at 11 57 41 AM

Let me check with the images .The images shouldn't be in base64.

The video link layout issue exists since our editor is only manually expandable.

@AbhishekPAnil
Copy link
Contributor Author

  1. When I publish, too many popups
    I get to success messages instead of one. You can reproduce it by saving any of my test events as draft and then republish.

cc @AbhishekPAnil Will solve this bug in this issue.

@troughc
Copy link
Contributor

troughc commented Aug 14, 2024

@AbhishekPAnil let's hold off on implementing this feature (i.e. adding buttons for images and video). Furthermore, we will not add the video option at all (remove that button completely).

Main point: this is on hold so I will unassign you and assign myself.

After discussing this with @saumier and I want to go back to the customer at Tout Culture to see if they can use our existing solution (image gallery). If they really want to keepimages in the descriptions, then we may implement this as a special feature that would only be available for Tout Culture (it would not be available in any other CMS for any other customer).

I will let you know the final decision.

@troughc
Copy link
Contributor

troughc commented Aug 14, 2024

Note that we will definitely be removing the ability to drag/drop or copy/paste images into descriptions. Stay tuned!

@AbhishekPAnil
Copy link
Contributor Author

  1. When I publish, too many popups
    I get to success messages instead of one. You can reproduce it by saving any of my test events as draft and then republish.

cc @AbhishekPAnil Will solve this bug in this issue.

We may have to treat this as a separate issue.
This needs debugging.This might have occured when we added the logic that when user changes the publish state of an event ,the modifier details shouldn't be updated.Creating duplicating event also has this issue.

@AbhishekPAnil
Copy link
Contributor Author

Note that we will definitely be removing the ability to drag/drop or copy/paste images into descriptions. Stay tuned!

I have updated the PR with the above changes.

@troughc
Copy link
Contributor

troughc commented Aug 20, 2024

Here is the new design for image gallery https://www.figma.com/design/DBnSSMT9Etfm6guDKKSp8O/Culture-Creates---Footlight-Calendar?node-id=9647-90907&t=ESgAxxnpWTxhlWcJ-1
to discuss

@troughc
Copy link
Contributor

troughc commented Aug 22, 2024

Hi @AbhishekPAnil I tested copy/paste and drag and drop of images into the description. This works, as in, nothing is getting added to the description. PASSED
Also tested that I could copy links: PASSED
I will hold onto this until i hear from Scott/Isabelle (do not deploy just yet).

@troughc
Copy link
Contributor

troughc commented Sep 11, 2024

@sahalali @AbhishekPAnil this is now approved to be deployed to PRODUCTION. thanks!

@troughc troughc removed their assignment Sep 11, 2024
@AbhishekPAnil
Copy link
Contributor Author

This is deployed in production

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request To be deployed Tout Culture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants