Skip to content

Commit

Permalink
Add isEditable property to ItemDetailState.Field struct
Browse files Browse the repository at this point in the history
  • Loading branch information
mvasilak committed Jan 13, 2025
1 parent adda28d commit c8209ea
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 40 deletions.
33 changes: 18 additions & 15 deletions Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ struct ItemDetailDataCreator {
/// - parameter doiDetector: DOI detector.
/// - parameter getExistingData: Closure for getting available data for given field. It passes the field key and baseField and receives existing
/// field name and value if available.
/// - returns: Tuple with 3 values: field keys of new fields, actual fields, `Bool` indicating whether this item type contains an abstract.
/// - returns: Tuple with 2 values: orderered dictionary of fields by field key, `Bool` indicating whether this item type contains an abstract.
static func fieldData(
for itemType: String,
schemaController: SchemaController,
Expand All @@ -208,12 +208,13 @@ struct ItemDetailDataCreator {
doiDetector: (String) -> Bool,
getExistingData: ((String, String?) -> (String?, String?))? = nil
) throws -> (OrderedDictionary<String, ItemDetailState.Field>, Bool) {
guard var fieldSchemas = schemaController.fields(for: itemType) else {
guard let fieldSchemas = schemaController.fields(for: itemType) else {
throw ItemDetailError.typeNotSupported(itemType)
}

var hasAbstract: Bool = false
let titleKey = schemaController.titleKey(for: itemType)
let isEditable = itemType != ItemTypes.attachment
var fields: OrderedDictionary<String, ItemDetailState.Field> = [:]
for schema in fieldSchemas {
let key = schema.field
Expand Down Expand Up @@ -242,29 +243,31 @@ struct ItemDetailDataCreator {
additionalInfo = [.formattedDate: Formatter.dateAndTime.string(from: date), .formattedEditDate: Formatter.sqlFormat.string(from: date)]
}

fields[key] = ItemDetailState.Field(key: key, baseField: baseField, name: name, value: value, isTitle: false, isTappable: isTappable, additionalInfo: additionalInfo)
fields[key] = ItemDetailState.Field(
key: key,
baseField: baseField,
name: name,
value: value,
isTitle: false,
isEditable: isEditable,
isTappable: isTappable,
additionalInfo: additionalInfo
)
}

return (fields, hasAbstract)
}

/// Returns all field keys for given item type, except those that should not appear as fields in item detail.
static func allFieldKeys(for itemType: String, schemaController: SchemaController) -> OrderedSet<String> {
guard let fieldSchemas = schemaController.fields(for: itemType) else { return [] }
var fieldKeys: OrderedSet<String> = OrderedSet(fieldSchemas.map({ $0.field }))
// Remove title and abstract keys, those 2 are used separately in Data struct.
fieldKeys.remove(FieldKeys.Item.abstract)
if let titleKey = schemaController.titleKey(for: itemType) {
fieldKeys.remove(titleKey)
}
return fieldKeys
}

/// Returns ordered set of keys for fields that have non-empty values.
static func nonEmptyFieldKeys(from fields: OrderedDictionary<String, ItemDetailState.Field>) -> OrderedSet<String> {
return fields.filter({ !$0.value.value.isEmpty }).keys
}

/// Returns ordered set of keys for fields that are either editable or have non-empty values.
static func editableOrNonEmptyFieldKeys(from fields: OrderedDictionary<String, ItemDetailState.Field>) -> OrderedSet<String> {
return fields.filter({ $0.value.isEditable || !$0.value.value.isEmpty }).keys
}

/// Checks whether field is tappable based on its key and value.
/// - parameter key: Key of field.
/// - parameter value: Value of field.
Expand Down
39 changes: 23 additions & 16 deletions Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct ItemDetailState: ViewModelState {
var name: String
var value: String
let isTitle: Bool
let isEditable: Bool
var isTappable: Bool
var additionalInfo: [AdditionalInfoKey: String]?

Expand Down Expand Up @@ -188,24 +189,30 @@ struct ItemDetailState: ViewModelState {
var maxNonemptyFieldTitleWidth: CGFloat = 0

func databaseFields(schemaController: SchemaController) -> [Field] {
var allFields = Array(self.fields.values)

if let titleKey = schemaController.titleKey(for: self.type) {
allFields.append(Field(key: titleKey,
baseField: (titleKey != FieldKeys.Item.title ? FieldKeys.Item.title : nil),
name: "",
value: self.title,
isTitle: true,
isTappable: false))
var allFields = Array(fields.values)

if let titleKey = schemaController.titleKey(for: type) {
allFields.append(Field(
key: titleKey,
baseField: (titleKey != FieldKeys.Item.title ? FieldKeys.Item.title : nil),
name: "",
value: title,
isTitle: true,
isEditable: !isAttachment,
isTappable: false
))
}

if let abstract = self.abstract {
allFields.append(Field(key: FieldKeys.Item.abstract,
baseField: nil,
name: "",
value: abstract,
isTitle: false,
isTappable: false))
if let abstract {
allFields.append(Field(
key: FieldKeys.Item.abstract,
baseField: nil,
name: "",
value: abstract,
isTitle: false,
isEditable: isAttachment,
isTappable: false
))
}

return allFields
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,11 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
if state.snapshot != nil || isEditing {
state.snapshot = data
}
if isEditing && !data.isAttachment {
state.presentedFieldIds = data.fields.keys
if isEditing {
// During editing present only editable fields or non-empty, non-editable ones.
state.presentedFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: data.fields)
} else {
// Otherwise present only non-empty fields.
state.presentedFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: data.fields)
}
state.attachments = attachments
Expand Down Expand Up @@ -648,9 +650,9 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
private func startEditing(in viewModel: ViewModel<ItemDetailActionHandler>) {
self.update(viewModel: viewModel) { state in
state.snapshot = state.data
if !state.data.isAttachment {
state.presentedFieldIds = ItemDetailDataCreator.allFieldKeys(for: state.data.type, schemaController: schemaController)
}
// state.data.fields has all available fields for this state.data.type,
// so we present only those that are editable or non-empty.
state.presentedFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: state.data.fields)
state.isEditing = true
state.changes.insert(.editing)
}
Expand Down Expand Up @@ -853,7 +855,9 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
self.update(viewModel: viewModel) { state in
if droppedFields.isEmpty {
state.data = itemData
state.presentedFieldIds = ItemDetailDataCreator.allFieldKeys(for: state.data.type, schemaController: schemaController)
// state.data.fields has all available fields for the changed state.data.type,
// so we present only those that are editable or non-empty.
state.presentedFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: state.data.fields)
state.changes.insert(.type)
} else {
// Notify the user, that some fields with values will be dropped
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ final class ItemDetailCollectionViewHandler: NSObject {

case .field(let key, let multiline):
guard let field = viewModel.state.data.fields[key] else { return collectionView.dequeueConfiguredReusableCell(using: emptyRegistration, for: indexPath, item: ()) }
if !isEditing || viewModel.state.data.isAttachment {
if !isEditing || !field.isEditable {
return collectionView.dequeueConfiguredReusableCell(using: fieldRegistration, for: indexPath, item: (.field(field), titleWidth))
}
if multiline {
Expand Down Expand Up @@ -917,8 +917,8 @@ extension ItemDetailCollectionViewHandler: UICollectionViewDelegate {
observer.on(.next(.openTypePicker))

case .field(let fieldId, _):
// Tappable fields should be only tappable when not in editing mode, in case of attachment, URL is not editable, so keep it tappable even while editing.
guard !viewModel.state.isEditing || (viewModel.state.data.type == ItemTypes.attachment), let field = viewModel.state.data.fields[fieldId], field.isTappable else { return }
// Tappable fields should be only tappable when not in editing mode, or field is not editable. E.g. in case of attachment, URL is not editable, so keep it tappable even while editing.
guard let field = viewModel.state.data.fields[fieldId], field.isTappable, !viewModel.state.isEditing || !field.isEditable else { return }

This comment has been minimized.

Copy link
@michalrentka

michalrentka Jan 24, 2025

Contributor

Now that I see this, isTappable and isEditable are mutually exclusive. Maybe we could have a simple enum with options tappable and editable, not sure if it wouldn't overcomplicate things, but I don't think so.

switch field.key {
case FieldKeys.Item.Attachment.url:
observer.on(.next(.openUrl(field.value)))
Expand Down

0 comments on commit c8209ea

Please sign in to comment.