Skip to content

feat(chat): Group the files of one upload into a single message - #2705

Open
Ivansss wants to merge 8 commits into
mainfrom
feat/noid/group-file-messages
Open

feat(chat): Group the files of one upload into a single message#2705
Ivansss wants to merge 8 commits into
mainfrom
feat/noid/group-file-messages

Conversation

@Ivansss

@Ivansss Ivansss commented Sep 10, 2026

Copy link
Copy Markdown
Member
IMG_E0377

Fixes #2701

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ivan Sein <ivan@nextcloud.com>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ivan Sein <ivan@nextcloud.com>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ivan Sein <ivan@nextcloud.com>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ivan Sein <ivan@nextcloud.com>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ivan Sein <ivan@nextcloud.com>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ivan Sein <ivan@nextcloud.com>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ivan Sein <ivan@nextcloud.com>
@SystemKeeper
SystemKeeper force-pushed the feat/noid/group-file-messages branch from 9b66e1c to a80e001 Compare September 10, 2026 15:59

@SystemKeeper SystemKeeper left a comment

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.

Just a first look at the first files.

Do we handle edit and delete of individual messages correctly?
I generally find this PR very verbose comment wise ..

groupedFilePreviewView.setup(with: group, account: account, availableWidth: self.availableBodyWidth)

// The caption is carried by the file shared last, which is what the group is shown as
let hasCaption = !group.anchor.sharesFileWithoutCaption

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.

I wonder if we can rely on that entirely? It’s up to the uploading client, isn’t it?

What happens with old clients? What happens with reaction on individual messages or captions (which is possible on old clients and api in general).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I wonder if we can rely on that entirely? It’s up to the uploading client, isn’t it?

It is up to the client yes, but if the client is sending the group reference id (supporting grouping feature) I think we can assume the caption will be on the last message.

What happens with old clients? What happens with reaction on individual messages or captions (which is possible on old clients and api in general).

This is a current issue, at the moment only the reactions of the last message of the group will be shown.
nextcloud/spreed#19371

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.

I think we can assume the caption will be on the last message.

But if the API/old clients can edit message in between, that's still an issue?


/// Whether the file is shown as a preview tile rather than as a row with its name.
///
/// Only media the server can render a preview of: everything else would be a tile showing the

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.

That is not true, but might still be intended. But the server is able to generate previews for pdfs or office documents if setup correctly

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's right, the comment needs to be updated but the functionality is intended (only images and video previews will be shown in the preview tiles)


/// The characters the agreed format allows, which is stricter than what a hash could contain:
/// a reference id with an uppercase letter in it is not one of ours.
private static let hexDigits = Set("0123456789abcdef")

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.

Why?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The reference id format is defined as /[a-f0-9]{60}-[0-9]{3}/ in nextcloud/spreed#19040
Do you mean why implementing it like this and not with a regex?
I would update the comment since it's a bit confusing.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ivan Sein <ivan@nextcloud.com>
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.

Multiple files sharing support 🖼️+🖼️

2 participants