Skip to content

[trello.com/c/4FomJzhO] Add Copy action to Failed messages menu + Open menu on long tap#833

Open
Lainaaa wants to merge 7 commits into
developfrom
trello.com/c/4FomJzhO
Open

[trello.com/c/4FomJzhO] Add Copy action to Failed messages menu + Open menu on long tap#833
Lainaaa wants to merge 7 commits into
developfrom
trello.com/c/4FomJzhO

Conversation

@Lainaaa

@Lainaaa Lainaaa commented Apr 22, 2025

Copy link
Copy Markdown
Member

​While working on the task, I noticed some shortcomings that initially seemed minor. However, due to the cell system, they turned out to be significant, leading to a considerable amount of boilerplate code. I hope I managed to reduce this boilerplate by extracting the copy functionality and the animated press effect, introduced by Vladimir, into separate methods. Additionally, I aimed to improve the code by relocating functions unrelated to ChatMenuManager into the cell.

@Lainaaa Lainaaa self-assigned this Apr 22, 2025
@art-divin

Copy link
Copy Markdown
Collaborator

/run-tests

Lainaaa added 5 commits April 23, 2025 14:18
# Conflicts:
#	Adamant/Modules/Chat/View/Subviews/ChatBaseMessage/ChatMessageCell.swift
#	Adamant/Modules/Chat/View/Subviews/ChatMedia/ChatMediaCell.swift
#	Adamant/Modules/Chat/View/Subviews/ChatReply/ChatMessageReplyCell.swift
#	Adamant/Modules/Chat/View/Subviews/ChatTransaction/Container/ChatTransactionContainerView.swift
@Lainaaa Lainaaa marked this pull request as ready for review April 24, 2025 12:32
@adamantmm

Copy link
Copy Markdown
Member

Don’t merge to dev it because of many changes.

It’s for release v3.12.

@art-divin art-divin requested a review from Copilot May 28, 2025 20:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the copy functionality and long press gesture handling across multiple chat components to reduce boilerplate code while adding support for failed message interactions.

  • Extracts copy-to-pasteboard behavior into shared dialog methods.
  • Consolidates long press gesture handling via a common GestureHelper interface.
  • Updates property names and visibility modifiers to better reflect intended usage.

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
CommonKit/Helpers/TaskManager.swift Made TaskManager API public and updated header comments.
CommonKit/Helpers/GestureHelper.swift Introduced a protocol extension for processing long press gestures.
AdamantDialogService.swift & DialogService.swift Added and integrated a new copyToPasteboard method.
PartnerQRViewModel.swift, ChatViewModel.swift Updated copy actions to use the new dialogService methods.
Various Chat module files Refactored long press and copy gesture handling to use copyAction closures and GestureHelper.
ChatMenuManager.swift, ChatDialogManager.swift, ChatDataSourceManager.swift, ChatAction.swift, ChatFactory.swift Updated state management and action handling for failed message scenarios and consistent copy actions.
Comments suppressed due to low confidence (1)

Adamant/Modules/Chat/View/Subviews/ChatTransaction/Container/ChatTransactionContainerView.swift:133

  • [nitpick] Property names in Swift should follow lowerCamelCase. Consider renaming 'GestureTaskManager' to 'gestureTaskManager' for consistency with Swift style guidelines.
var GestureTaskManager: TaskManager = TaskManager()

Comment on lines +161 to +162
self?.cellContainerView.animatePressDown() },
onGestureEnded: { [weak self] in self?.messageContainerView.animatePressUp() }

Copilot AI May 28, 2025

Copy link

Choose a reason for hiding this comment

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

The onGestureEnded closure in handleLongPressToOpenMenu references 'messageContainerView', but ChatMediaCell defines 'cellContainerView'. Please update it to use the correct property for consistency.

Suggested change
self?.cellContainerView.animatePressDown() },
onGestureEnded: { [weak self] in self?.messageContainerView.animatePressUp() }
self?.cellContainerView.animatePressDown() },
onGestureEnded: { [weak self] in self?.cellContainerView.animatePressUp() }

Copilot uses AI. Check for mistakes.

extension ChatMessageCell: ChatMenuManagerDelegate {
var isFailedMessage: Bool {
model.backgroundColor == .failed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Generally speaking ,deciding on the state based on backgroundColor is not a future-proof solution. Would you consider improving this part a bit?

model.backgroundColor == .failed
}

func showFailedMenu(){

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

showFailedMenuAlert ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants