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

feat: "Feature Request: Bulk Image Upload Capability in Media Popup" #398

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"WillLuke.nextjs.hasPrompted": true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file from the PR.

}
86 changes: 86 additions & 0 deletions apps/frontend/src/components/MediaPopup.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import React, { useState } from 'react';

interface MediaPopupProps {
images: string[]; // List of image URLs or file references
onAddImages: (selectedImages: string[]) => void; // Callback for selected images
}

const MediaPopup: React.FC<MediaPopupProps> = ({ images, onAddImages }) => {
const [selectedImages, setSelectedImages] = useState<string[]>([]);

// Toggle the selection of an image
const toggleImageSelection = (image: string) => {
if (selectedImages.includes(image)) {
setSelectedImages(selectedImages.filter((img) => img !== image)); // Remove if already selected
} else {
setSelectedImages([...selectedImages, image]); // Add to selected images
}
};

// Handle the add button click
const handleAddClick = () => {
onAddImages(selectedImages); // Pass selected images back to the parent
setSelectedImages([]); // Clear selection after adding
};

return (
<div className="media-popup">
<h3>Select Images</h3>
<div className="image-grid">
{images.map((image) => (
<div
key={image}
className={`image-item ${
selectedImages.includes(image) ? 'selected' : ''
}`}
onClick={() => toggleImageSelection(image)}
>
<img src={image} alt="media" />

Check warning

Code scanning / ESLint

Prevent usage of `<img>` element due to slower LCP and higher bandwidth. Warning

Using <img> could result in slower LCP and higher bandwidth. Consider using <Image /> from next/image to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Replace <img> with Next.js Image component.

Similar to the previous file, replace the <img> element with Next.js's Image component for better performance.

+import Image from 'next/image';

-            <img src={image} alt="media" />
+            <Image
+              src={image}
+              alt="media"
+              width={100}
+              height={100}
+              objectFit="cover"
+            />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<img src={image} alt="media" />
import Image from 'next/image';
...
<Image
src={image}
alt="media"
width={100}
height={100}
objectFit="cover"
/>
...
🧰 Tools
🪛 GitHub Check: ESLint

[warning] 38-38: Prevent usage of <img> element due to slower LCP and higher bandwidth.
Using <img> could result in slower LCP and higher bandwidth. Consider using <Image /> from next/image to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element

</div>
))}
</div>
<button onClick={handleAddClick} disabled={selectedImages.length === 0}>
Add {selectedImages.length > 0 ? `(${selectedImages.length})` : ''}{' '}
Images
</button>

<style jsx>{`
.media-popup {
padding: 20px;
}
.image-grid {
display: grid;
grid-template-columns: repeat(auto-fill, minmax(100px, 1fr));
gap: 10px;
}
.image-item {
cursor: pointer;
border: 2px solid transparent;
transition: border-color 0.3s ease;
}
.image-item img {
max-width: 100px;
height: auto;
}
.image-item.selected {
border-color: blue; /* Border for selected images */
}
button {
margin-top: 20px;
padding: 10px;
background-color: #007bff;
color: white;
border: none;
cursor: pointer;
border-radius: 5px;
}
button:disabled {
background-color: grey;
cursor: not-allowed;
}
`}</style>
</div>
Comment on lines +26 to +82
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add keyboard navigation and accessibility features.

The component should support keyboard navigation and include proper ARIA attributes for better accessibility.

   return (
-    <div className="media-popup">
+    <div
+      className="media-popup"
+      role="dialog"
+      aria-labelledby="media-popup-title"
+    >
-      <h3>Select Images</h3>
+      <h3 id="media-popup-title">Select Images</h3>
       <div className="image-grid">
         {images.map((image) => (
           <div
             key={image}
             className={`image-item ${
               selectedImages.includes(image) ? 'selected' : ''
             }`}
             onClick={() => toggleImageSelection(image)}
+            onKeyDown={(e) => {
+              if (e.key === 'Enter' || e.key === ' ') {
+                toggleImageSelection(image);
+              }
+            }}
+            role="button"
+            tabIndex={0}
+            aria-pressed={selectedImages.includes(image)}
           >
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return (
<div className="media-popup">
<h3>Select Images</h3>
<div className="image-grid">
{images.map((image) => (
<div
key={image}
className={`image-item ${
selectedImages.includes(image) ? 'selected' : ''
}`}
onClick={() => toggleImageSelection(image)}
>
<img src={image} alt="media" />
</div>
))}
</div>
<button onClick={handleAddClick} disabled={selectedImages.length === 0}>
Add {selectedImages.length > 0 ? `(${selectedImages.length})` : ''}{' '}
Images
</button>
<style jsx>{`
.media-popup {
padding: 20px;
}
.image-grid {
display: grid;
grid-template-columns: repeat(auto-fill, minmax(100px, 1fr));
gap: 10px;
}
.image-item {
cursor: pointer;
border: 2px solid transparent;
transition: border-color 0.3s ease;
}
.image-item img {
max-width: 100px;
height: auto;
}
.image-item.selected {
border-color: blue; /* Border for selected images */
}
button {
margin-top: 20px;
padding: 10px;
background-color: #007bff;
color: white;
border: none;
cursor: pointer;
border-radius: 5px;
}
button:disabled {
background-color: grey;
cursor: not-allowed;
}
`}</style>
</div>
return (
<div
className="media-popup"
role="dialog"
aria-labelledby="media-popup-title"
>
<h3 id="media-popup-title">Select Images</h3>
<div className="image-grid">
{images.map((image) => (
<div
key={image}
className={`image-item ${selectedImages.includes(image) ? 'selected' : ''}`}
onClick={() => toggleImageSelection(image)}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
toggleImageSelection(image);
}
}}
role="button"
tabIndex={0}
aria-pressed={selectedImages.includes(image)}
>
<img src={image} alt="media" />
</div>
))}
</div>
<button onClick={handleAddClick} disabled={selectedImages.length === 0}>
Add {selectedImages.length > 0 ? `(${selectedImages.length})` : ''} Images
</button>
<style jsx>{`
.media-popup {
padding: 20px;
}
.image-grid {
display: grid;
grid-template-columns: repeat(auto-fill, minmax(100px, 1fr));
gap: 10px;
}
.image-item {
cursor: pointer;
border: 2px solid transparent;
transition: border-color 0.3s ease;
}
.image-item img {
max-width: 100px;
height: auto;
}
.image-item.selected {
border-color: blue; /* Border for selected images */
}
button {
margin-top: 20px;
padding: 10px;
background-color: #007bff;
color: white;
border: none;
cursor: pointer;
border-radius: 5px;
}
button:disabled {
background-color: grey;
cursor: not-allowed;
}
`}</style>
</div>
);
🧰 Tools
🪛 GitHub Check: ESLint

[warning] 38-38: Prevent usage of <img> element due to slower LCP and higher bandwidth.
Using <img> could result in slower LCP and higher bandwidth. Consider using <Image /> from next/image to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element

);
};

export default MediaPopup;
60 changes: 60 additions & 0 deletions apps/frontend/src/components/PostCreator.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import React, { useState } from 'react';
import MediaPopup from './MediaPopup'; // Import the MediaPopup component

const PostCreator: React.FC = () => {
const [showMediaPopup, setShowMediaPopup] = useState(false);
const [selectedImages, setSelectedImages] = useState<string[]>([]);

const imageGallery = [
'image1.jpg',
'image2.jpg',
'image3.jpg',
'image4.jpg',
// Add more image URLs or file paths here
];

// Callback to handle the images selected from the media popup
const handleAddImages = (images: string[]) => {
setSelectedImages((prevImages) => [...prevImages, ...images]); // Add the selected images to the existing list
setShowMediaPopup(false); // Close the popup after adding
};

return (
<div>
<h1>Create a Post</h1>
<button onClick={() => setShowMediaPopup(true)}>Add Images</button>

{/* Display selected images */}
<div className="selected-images">
{selectedImages.map((image) => (
<img
key={image}
src={image}
alt="Selected"
className="selected-image"
/>
Comment on lines +30 to +35

Check warning

Code scanning / ESLint

Prevent usage of `<img>` element due to slower LCP and higher bandwidth. Warning

Using <img> could result in slower LCP and higher bandwidth. Consider using <Image /> from next/image to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element
))}
Comment on lines +29 to +36
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Replace <img> with Next.js Image component for better performance.

Using the <img> element can result in slower LCP (Largest Contentful Paint) and higher bandwidth usage. Consider using Next.js's Image component for automatic image optimization.

+import Image from 'next/image';

-        {selectedImages.map((image) => (
-          <img
-            key={image}
-            src={image}
-            alt="Selected"
-            className="selected-image"
-          />
-        ))}
+        {selectedImages.map((image) => (
+          <Image
+            key={image}
+            src={image}
+            alt="Selected"
+            width={100}
+            height={100}
+            className="selected-image"
+            objectFit="cover"
+          />
+        ))}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{selectedImages.map((image) => (
<img
key={image}
src={image}
alt="Selected"
className="selected-image"
/>
))}
import Image from 'next/image';
// ... other imports and code above remain unchanged
{selectedImages.map((image) => (
<Image
key={image}
src={image}
alt="Selected"
width={100}
height={100}
className="selected-image"
objectFit="cover"
/>
))}
🧰 Tools
🪛 GitHub Check: ESLint

[warning] 30-35: Prevent usage of <img> element due to slower LCP and higher bandwidth.
Using <img> could result in slower LCP and higher bandwidth. Consider using <Image /> from next/image to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element

</div>

{/* Media popup */}
{showMediaPopup && (
<MediaPopup images={imageGallery} onAddImages={handleAddImages} />
)}

<style jsx>{`
.selected-images {
margin-top: 20px;
display: flex;
gap: 10px;
}
.selected-image {
max-width: 100px;
height: auto;
border: 2px solid #ddd;
}
`}</style>
</div>
);
};

export default PostCreator;
Loading