Skip to content

[stack] feat(sharing): use the new email blocks for share and note mails - #65220

Merged
skjnldsv merged 1 commit into
masterfrom
feature/sharing-mail-blocks
Oct 7, 2026
Merged

skjnldsv merged 1 commit into
masterfrom
feature/sharing-mail-blocks

Conversation

@skjnldsv

@skjnldsv skjnldsv commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Summary

Builds on #65195 and gives the other sharing mails the same treatment as share-by-mail.

Mail Now shows
User share Sharer, note box, card with the file and its expiry date (which was never shown before)
Note added to a share Sharer and note box, for both user and mail shares
Upload reminder Card with the folder and its expiry date

New strings: "Note", "Valid until".

Before After (light) After (dark) Mobile
User share
Note, user share
Note, mail share
Upload reminder

Browser renders of the real code with sample data, after includes the restyle from #65194. Not checked in mail clients.

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests are included
  • Screenshots before/after for front-end changes
  • Documentation has been updated or is not required
  • Backports requested where applicable
  • Labels added where applicable
  • Milestone added for target branch/version

AI (if applicable)

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

👾 This pull request was assisted by Claude Code, commits carry an Assisted-by trailer.

@skjnldsv skjnldsv added this to the Nextcloud 36 milestone Oct 6, 2026
@skjnldsv skjnldsv self-assigned this Oct 6, 2026
@skjnldsv
skjnldsv force-pushed the feature/sharing-mail-blocks branch from bd68bea to 311a3a4 Compare October 6, 2026 18:15
@skjnldsv
skjnldsv force-pushed the feature/sharing-mail-blocks branch from 311a3a4 to 2477813 Compare October 6, 2026 18:23
@skjnldsv skjnldsv changed the title feat(sharing): use the new email blocks for share and note mails [stack] feat(sharing): use the new email blocks for share and note mails Oct 6, 2026
@skjnldsv
skjnldsv changed the base branch from feature/mail-template-blocks to feature/sharebymail-mail-blocks October 6, 2026 18:24
@skjnldsv
skjnldsv added this pull request to stack #65223 October 6, 2026 18:34
@skjnldsv
skjnldsv marked this pull request as ready for review October 6, 2026 18:35
@skjnldsv
skjnldsv requested a review from a team as a code owner October 6, 2026 18:35
@skjnldsv skjnldsv removed the 2. developing Work in progress label Oct 6, 2026
@skjnldsv
skjnldsv requested review from Altahrim, leftybournes, provokateurin and salmart-dev and removed request for a team October 6, 2026 18:35
@skjnldsv skjnldsv added the 3. to review Waiting for reviews label Oct 6, 2026
@skjnldsv
skjnldsv force-pushed the feature/sharing-mail-blocks branch 2 times, most recently from 98a75c8 to 4e2cfd0 Compare October 7, 2026 08:25
Base automatically changed from feature/sharebymail-mail-blocks to master October 7, 2026 09:29
@skjnldsv
skjnldsv requested a review from susnux October 7, 2026 09:30
@skjnldsv
skjnldsv force-pushed the feature/sharing-mail-blocks branch from 4e2cfd0 to 038df60 Compare October 7, 2026 09:34
Comment thread lib/private/Share20/DefaultShareProvider.php Outdated

if ($note !== '') {
$emailTemplate->addBodyText(htmlspecialchars($note), $note);
$emailTemplate->addBodyNote($note, $l->t('Note from %s', [$initiatorDisplayName]));

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.

It looks like every call to addBodyNote does this translation/concatenation, should it be done inside addBodyNote and it takes an author parameter instead?
Same kind of question for EmailDetails and Valid until.

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.

Hmm, I kept them in the callers on purpose: the template doesn't know the recipient's language (or locale for the date), the providers do. But you're right about the repetition, I'll move both into a helper in DefaultShareProvider that sharebymail reuses.

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.

Update: with only the label now just 'Note' (per Jan's review), the repeated part shrank to a single t('Note') call, so I skipped the helper. Happy to add it if you still want it

jancborchardt

This comment was marked as resolved.

@skjnldsv

This comment was marked as resolved.

@jancborchardt

Copy link
Copy Markdown
Member

@skjnldsv ok, then do we wanna instead remove the label "Note from [Name]", or reduce it to simply "Note"? And the "and wants to add" can definitely go from the text. :)

@skjnldsv

skjnldsv commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

@skjnldsv ok, then do we wanna instead remove the label "Note from [Name]", or reduce it to simply "Note"? And the "and wants to add" can definitely go from the text. :)

Both are fine to me !
I'll adjust right away

User share mails now show the sharer, the note in a note box and a
details card with the file name and its expiration date. Note mails
show the sharer and the note box, and the upload reminder shows the
folder and its expiration date in a details card.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: John Molakvoæ <14975046+skjnldsv@users.noreply.github.com>
@skjnldsv
skjnldsv force-pushed the feature/sharing-mail-blocks branch from 038df60 to 10ea9a1 Compare October 7, 2026 16:42
@skjnldsv
skjnldsv requested a review from come-nc October 7, 2026 17:52
@skjnldsv
skjnldsv merged commit 128134f into master Oct 7, 2026
180 checks passed
@skjnldsv
skjnldsv deleted the feature/sharing-mail-blocks branch October 7, 2026 17:54
@skjnldsv

skjnldsv commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Went with "Note" as the label and dropped "and wants to add", in all share mails including share by mail 👍

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants