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

dev: Add drag and drop, fixes and review are still needed #78

Merged
merged 3 commits into from
Jan 29, 2025
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
<key>All.xcscheme_^#shared#^_</key>
<dict>
<key>orderHint</key>
<integer>2</integer>
<integer>0</integer>
</dict>
<key>sources.xcscheme_^#shared#^_</key>
<dict>
<key>orderHint</key>
<integer>0</integer>
<integer>1</integer>
</dict>
</dict>
</dict>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<key>PhotoBook.xcscheme_^#shared#^_</key>
<dict>
<key>orderHint</key>
<integer>1</integer>
<integer>2</integer>
</dict>
</dict>
</dict>
Expand Down
9 changes: 9 additions & 0 deletions osx/PhotoBook/PhotoBook/Media/MediaList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ class MediaListModel: ObservableObject
self.photobook.removeImportFolder(selectedItem?.path)
}
}

public func selectedIndex() -> UInt?
{
if let index = list.firstIndex(where: {$0.path == selectedItem?.path})
{
return UInt(index)
}
return nil
}
}

struct MediaList: View
Expand Down
2 changes: 2 additions & 0 deletions osx/PhotoBook/PhotoBook/Pages/DashboardView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,6 @@ struct DashboardView: View, PhotobookUIListener {
func onImageUpdated(root: String, row:UInt, index:UInt){}

func onCollageThumbnailsCreated(){}

func onImageMapped(imageId: String, image: FrontendImage){}
}
6 changes: 6 additions & 0 deletions osx/PhotoBook/PhotoBook/Pages/PhotoBookApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ private var noirUIListener: [NoirUIListener] = []
{
photobookUIListener.last?.onCollageThumbnailsCreated()
}

func onImageMapped(_ imageId: String, image: FrontendImage)
{
photobookUIListener.last?.onImageMapped(imageId:imageId, image: image)
}
}

@objc extension NoirListenerWrapperCLevel
Expand Down Expand Up @@ -107,6 +112,7 @@ struct PhotoBookApp: App, PhotobookUIListener, NoirUIListener {
func onMappingFinished(root: String) {}
func onImageUpdated(root: String, row:UInt, index:UInt){}
func onCollageThumbnailsCreated(){}
func onImageMapped(imageId: String, image: FrontendImage){}
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Remove duplicate protocol implementation

The empty onImageMapped implementation at line 115 is redundant as PhotoBookApp already implements this method at line 39 with proper delegation logic. Having two implementations of the same protocol method within the same type can lead to unexpected behavior.

  • Remove the empty implementation at line 115 since the working implementation at line 39 already handles the protocol requirement by delegating to the last listener.
🔗 Analysis chain

Review empty implementation and verify protocol requirements.

The empty implementation raises several concerns:

  1. The TODO comment above suggests that PhotobookUIListener might not be needed in this struct
  2. If the listener is needed, the empty implementation should be documented or handle state updates

Let's verify the protocol usage:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for PhotobookUIListener protocol definition and its usage
ast-grep --pattern 'protocol PhotobookUIListener {
  $$$
}'

# Search for other implementations of onImageMapped
rg -p "func onImageMapped.*FrontendImage" -A 5

Length of output: 952

func onNoirLutAdded(item:LutItem) {
lutGridModel.images.append(item)
}
Expand Down
60 changes: 55 additions & 5 deletions osx/PhotoBook/PhotoBook/Pages/TableContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//

import SwiftUI
import UniformTypeIdentifiers

struct TableContentView: View, PhotobookUIListener {
@State private var navigateToDashboard = false
Expand All @@ -18,6 +19,7 @@ struct TableContentView: View, PhotobookUIListener {
@State private var showImportMediaPicker = false

@State private var uplModel: UnstagedPhotoLineModel = UnstagedPhotoLineModel()
@State private var splModel: StagedPhotoLineModel = StagedPhotoLineModel()

@State private var mediaListModel: MediaListModel

Expand Down Expand Up @@ -264,17 +266,55 @@ struct TableContentView: View, PhotobookUIListener {
VStack {
ScrollView(.horizontal, showsIndicators: false) {
HStack {

StagedPhotoLine(model: splModel, canvasImage: $canvasModel.mainImage)
}
.frame(width:geometry.size.width, height: 80)
.border(Color.BorderColor, width: 1)
.onDrop(of: [.text], isTargeted: nil) { providers in
guard let provider = providers.first else { return false}
if provider.hasItemConformingToTypeIdentifier(UTType.plainText.identifier) {
provider.loadDataRepresentation(forTypeIdentifier: UTType.plainText.identifier) { data, error in
if let data = data {
do {
let uplIdentifier = try self.decodeData(data)
var images: [String: FrontendImage] = [:]
for index in uplIdentifier.indices
{
if let row = uplIdentifier.row
{
let image = self.photobook.projectManagementService().unstagedImagesRepo().image(UInt32(row), index:UInt32(index))

let uuid = UUID()
images[uuid.uuidString] = image
}
else
{
assert(false, "Unreachable code")
}
}
self.photobook.mapImages(toSPL: images)

} catch {
print("Failed to decode dropped data: \(error)")
}
} else if let error = error {
assert(false, "Unreachable code \(error)")
}
else
{
assert(false, "Unreachable code")
}
}
}
print("Item dropped")
return true
}
.padding(.horizontal)
.frame(minHeight:80)
}

UnstagedPhotoLine(model: uplModel, canvasImage: $canvasModel.mainImage)
UnstagedPhotoLine(model: uplModel, canvasImage: $canvasModel.mainImage, mediaListModel: $mediaListModel)

}
.frame(height: geometry.size.height * 0.3)
.border(Color.BorderColor, width: 1)
}
}
.onAppear()
Expand All @@ -291,6 +331,11 @@ struct TableContentView: View, PhotobookUIListener {
.background(Color.PrimaryColor)

}
func decodeData(_ data: Data) throws -> UPLIdentifier {
let decoder = JSONDecoder()
let decoded = try decoder.decode(UPLIdentifier.self, from: data)
return decoded
}

func onProjectRead(){}
func onMetadataUpdated(focusedName: String){}
Expand Down Expand Up @@ -337,5 +382,10 @@ struct TableContentView: View, PhotobookUIListener {
}
}
}

func onImageMapped(imageId: String, image: FrontendImage)
{
self.splModel.list.append(image)
}
}

3 changes: 3 additions & 0 deletions osx/PhotoBook/PhotoBook/Photobook.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "PaperSettings.h"
#include "CollageItem.h"
#include "LutItem.h"
#include "FrontendImage.h"
#include "ProjectMetadataEntry.h"
#include "ProjectManagementService.h"

Expand All @@ -20,6 +21,7 @@
- (void)onMappingFinished:(NSString*)root;
- (void)onImageUpdated:(NSString*)root row:(unsigned)row index:(unsigned)index;
- (void)onCollageThumbnailsCreated;
- (void)onImageMapped:(NSString*)imageId image:(FrontendImage*)image;
@end

@interface NoirListenerWrapperCLevel: NSObject
Expand All @@ -45,6 +47,7 @@
- (void) addImportFolder:(NSString*)root;
- (void) removeImportFolder:(NSString*)root;
- (NSArray<CollageItem*>*) collageTemplatesThumbnailsList;
- (void) mapImagesToSPL:(NSDictionary<NSString*, FrontendImage*>*)images;
@end

#endif /* Photobook_h */
37 changes: 35 additions & 2 deletions osx/PhotoBook/PhotoBook/Photobook.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#import <Foundation/Foundation.h>

#include <memory>
#include <unordered_map>

#include <pb/PhotoBook.h>
#include <pb/entities/LutIconInfo.h>
Expand Down Expand Up @@ -80,8 +81,14 @@ void post(std::function<void()> f) override {
});
}
void onCollageCreated(unsigned index, PB::GenericImagePtr newImage) override {}
void onImageMapped(PBDev::ImageToPaperId id,
PB::GenericImagePtr image) override {}
void onImageMapped(PBDev::ImageToPaperId imageId,
PB::GenericImagePtr image) override {
std::string imageIdStr = boost::uuids::to_string(*imageId);
NSString* managedImageId = [NSString stringWithUTF8String:imageIdStr.c_str()];

auto managedImage = [[FrontendImage alloc] initWithCpp:image];
[&mManagedListener onImageMapped:managedImageId image:managedImage];
}
void onProgressUpdate(PB::ProgressStatus status) override {}

[[deprecated]]
Expand Down Expand Up @@ -212,4 +219,30 @@ - (void) removeImportFolder:(NSString*)root
return [list copy];
}

- (void) mapImagesToSPL:(NSDictionary<NSString*, FrontendImage*>*)images
{
auto imageToPaperService = mPhotobook->imageToPaperService();

std::unordered_map<PBDev::ImageToPaperId, PB::GenericImagePtr,
boost::hash<PBDev::ImageToPaperId>>
backendMap;

for (NSString *key in images) {
std::string uuidStr = [key UTF8String];
try {
boost::uuids::string_generator gen;
boost::uuids::uuid nativeUuid = gen(uuidStr);

PBDev::ImageToPaperId imageId = PBDev::ImageToPaperId(nativeUuid);

backendMap[imageId] = [images[key] unwrap];

} catch (const std::exception& e) {
}
}

imageToPaperService->map(
PBDev::ImageToPaperServiceId(PB::RuntimeUUID::newUUID()), backendMap);
}
Comment on lines +222 to +246
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling and validation in mapImagesToSPL.

The implementation has several issues:

  1. Empty catch block silently ignores UUID parsing errors
  2. No validation for null images
  3. No error propagation to Swift layer
 - (void) mapImagesToSPL:(NSDictionary<NSString*, FrontendImage*>*)images
 {
+    if (!images) {
+        NSLog(@"Error: images dictionary is null");
+        return;
+    }
     auto imageToPaperService = mPhotobook->imageToPaperService();
     
     std::unordered_map<PBDev::ImageToPaperId, PB::GenericImagePtr,
                          boost::hash<PBDev::ImageToPaperId>>
           backendMap;
     
     for (NSString *key in images) {
+        if (!images[key]) {
+            NSLog(@"Error: null image for key %@", key);
+            continue;
+        }
         std::string uuidStr = [key UTF8String];
         try {
             boost::uuids::string_generator gen;
             boost::uuids::uuid nativeUuid = gen(uuidStr);
             
             PBDev::ImageToPaperId imageId = PBDev::ImageToPaperId(nativeUuid);
             
             backendMap[imageId] = [images[key] unwrap];
             
         } catch (const std::exception& e) {
+            NSLog(@"Error parsing UUID %@: %s", key, e.what());
         }
     }
📝 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
- (void) mapImagesToSPL:(NSDictionary<NSString*, FrontendImage*>*)images
{
auto imageToPaperService = mPhotobook->imageToPaperService();
std::unordered_map<PBDev::ImageToPaperId, PB::GenericImagePtr,
boost::hash<PBDev::ImageToPaperId>>
backendMap;
for (NSString *key in images) {
std::string uuidStr = [key UTF8String];
try {
boost::uuids::string_generator gen;
boost::uuids::uuid nativeUuid = gen(uuidStr);
PBDev::ImageToPaperId imageId = PBDev::ImageToPaperId(nativeUuid);
backendMap[imageId] = [images[key] unwrap];
} catch (const std::exception& e) {
}
}
imageToPaperService->map(
PBDev::ImageToPaperServiceId(PB::RuntimeUUID::newUUID()), backendMap);
}
- (void) mapImagesToSPL:(NSDictionary<NSString*, FrontendImage*>*)images
{
if (!images) {
NSLog(@"Error: images dictionary is null");
return;
}
auto imageToPaperService = mPhotobook->imageToPaperService();
std::unordered_map<PBDev::ImageToPaperId, PB::GenericImagePtr,
boost::hash<PBDev::ImageToPaperId>>
backendMap;
for (NSString *key in images) {
if (!images[key]) {
NSLog(@"Error: null image for key %@", key);
continue;
}
std::string uuidStr = [key UTF8String];
try {
boost::uuids::string_generator gen;
boost::uuids::uuid nativeUuid = gen(uuidStr);
PBDev::ImageToPaperId imageId = PBDev::ImageToPaperId(nativeUuid);
backendMap[imageId] = [images[key] unwrap];
} catch (const std::exception& e) {
NSLog(@"Error parsing UUID %@: %s", key, e.what());
}
}
imageToPaperService->map(
PBDev::ImageToPaperServiceId(PB::RuntimeUUID::newUUID()), backendMap);
}


@end
1 change: 1 addition & 0 deletions osx/PhotoBook/PhotoBook/PhotobookUIListener.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ protocol PhotobookUIListener
func onMappingFinished(root: String)
func onImageUpdated(root: String, row:UInt, index:UInt)
func onCollageThumbnailsCreated()
func onImageMapped(imageId: String, image: FrontendImage)
}
50 changes: 50 additions & 0 deletions osx/PhotoBook/PhotoBook/SPL/StagedPhotoLine.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//
// StagedPhotoLine.swift
// PhotoBook
//
// Created by Cosmin Mihai on 29.01.2025.
//

import SwiftUI

class StagedPhotoLineModel: ObservableObject
{
@Published public var list: [FrontendImage] = []
}

struct StagedPhotoLine: View
{
@ObservedObject var model: StagedPhotoLineModel
@Binding var canvasImage: FrontendImage?
@State var selectedIndex: Int = -1

var body: some View {
ScrollView(.horizontal, showsIndicators: false) {
HStack {
ForEach(self.model.list.indices, id: \.self) { index in
if let fileName = self.model.list[index].resources().small
{
if let nsImage = NSImage(contentsOfFile: fileName) {
Image(nsImage: nsImage)
.cornerRadius(10)
.frame(height: 80)
.overlay(
RoundedRectangle(cornerRadius: 10)
.stroke(selectedIndex == index ? Color.yellow : Color.clear, lineWidth: 3)
)
.onTapGesture {
self.canvasImage = model.list[index]
selectedIndex = index
}
} else {
Text("Image not found")
}
}
}
}
.padding(.horizontal)
.frame(minHeight:80)
.border(Color.BorderColor, width: 1)
}
}
}
37 changes: 37 additions & 0 deletions osx/PhotoBook/PhotoBook/UPL/UPLIdentifier.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//
// UPLIdentifier.swift
// PhotoBook
//
// Created by Cosmin Mihai on 26.01.2025.
//

import SwiftUI
import UniformTypeIdentifiers

class UPLIdentifier: NSObject, Identifiable, NSItemProviderWriting, Encodable, Decodable
{
public var id = UUID()
public var row:UInt?;
public var indices:[UInt];

init(row:UInt?, indices:[UInt])
{
self.row = row
self.indices = indices
}

static var writableTypeIdentifiersForItemProvider: [String] {
return [UTType.plainText.identifier]
}

func loadData(withTypeIdentifier typeIdentifier: String, forItemProviderCompletionHandler completionHandler: @escaping @Sendable (Data?, (any Error)?) -> Void) -> Progress? {
do {
let encoder = JSONEncoder()
let dataToReturn = try encoder.encode(self)
completionHandler(dataToReturn, nil)
} catch {
completionHandler(nil, error)
}
return Progress(totalUnitCount: 100)
}
Comment on lines +27 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

Improve progress reporting and error handling.

The current implementation has several issues:

  1. The Progress object doesn't reflect actual work done
  2. No validation of the type identifier
  3. Error handling could be more specific

Consider this improved implementation:

func loadData(withTypeIdentifier typeIdentifier: String, forItemProviderCompletionHandler completionHandler: @escaping @Sendable (Data?, (any Error)?) -> Void) -> Progress? {
    // Verify type identifier
    guard typeIdentifier == UTType.plainText.identifier else {
        completionHandler(nil, NSError(domain: "UPLIdentifier", code: 1, userInfo: [
            NSLocalizedDescriptionKey: "Unsupported type identifier: \(typeIdentifier)"
        ]))
        return nil
    }
    
    let progress = Progress(totalUnitCount: 100)
    
    do {
        let encoder = JSONEncoder()
        progress.completedUnitCount = 50 // Encoding started
        let dataToReturn = try encoder.encode(self)
        progress.completedUnitCount = 100 // Encoding completed
        completionHandler(dataToReturn, nil)
    } catch {
        completionHandler(nil, error)
    }
    
    return progress
}

}
5 changes: 5 additions & 0 deletions osx/PhotoBook/PhotoBook/UPL/UnstagedPhotoLine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ struct UnstagedPhotoLine: View
{
@ObservedObject var model: UnstagedPhotoLineModel
@Binding var canvasImage: FrontendImage?
@Binding var mediaListModel: MediaListModel
@State var selectedIndex: Int = -1

var body: some View {
Expand All @@ -36,6 +37,9 @@ struct UnstagedPhotoLine: View
self.canvasImage = model.list[index]
selectedIndex = index
}
.onDrag {
NSItemProvider(object: UPLIdentifier(row:mediaListModel.selectedIndex(), indices:[UInt(index)]))
}
} else {
Text("Image not found")
}
Expand All @@ -44,6 +48,7 @@ struct UnstagedPhotoLine: View
}
.padding(.horizontal)
.frame(minHeight:80)
.border(Color.BorderColor, width: 1)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ typedef NS_ENUM(NSInteger, VirtualImageType) {
#endif
- (VirtualImageType)imageType;
- (FrontendImageResources*)resources;
#if __cplusplus
- (PB::GenericImagePtr)unwrap;
#endif
@end


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//

#import <Foundation/Foundation.h>
#import <UniformTypeIdentifiers/UniformTypeIdentifiers.h>

#include "FrontendImage.h"

Expand Down Expand Up @@ -34,4 +35,9 @@ - (FrontendImageResources*)resources
return frontendImage;
}

- (PB::GenericImagePtr)unwrap
{
return cppImage;
}

@end
Loading
Loading