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.

Sign up for GitHub

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

Jump to bottom

fix(share): Don't print twice the same text on emails #49746

Merged
merged 1 commit into from
Dec 11, 2024
Merged

fix(share): Don't print twice the same text on emails #49746

merged 1 commit into from
Dec 11, 2024
+16 −31

Conversation

solracsf
Copy link
Member

@solracsf solracsf commented Dec 10, 2024 •
edited
Loading

Summary

Don't print twice the same text on email.

Also removed those » « From DefaultShare as they are not printed on ShareByMail for consistency.

Before

image

After

image

Checklist

@solracsf solracsf added this to the Nextcloud 31 milestone Dec 10, 2024
@solracsf solracsf added 3. to review Waiting for reviews feature: emails labels Dec 10, 2024
@solracsf solracsf force-pushed the printOnlyOnceText branch 2 times, most recently from ef76037 to 0d8e98d Compare December 10, 2024 09:16
@solracsf solracsf marked this pull request as ready for review December 10, 2024 09:54
@solracsf solracsf requested review from skjnldsv, come-nc and a team December 10, 2024 09:54
@solracsf solracsf changed the title fix(share): Don't print twice the same information fix(share): Don't print twice the same information on emails Dec 10, 2024
@solracsf solracsf changed the title fix(share): Don't print twice the same information on emails fix(share): Don't print twice the same text on emails Dec 10, 2024
@solracsf
Copy link
Member Author

CI seems broken because of #49693 😢

@skjnldsv
Copy link
Member

skjnldsv commented Dec 10, 2024 •
edited
Loading

Rebased, cypress passes now

@solracsf solracsf added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 10, 2024
@solracsf solracsf requested a review from come-nc December 10, 2024 18:22
@skjnldsv skjnldsv merged commit f476265 into master Dec 11, 2024
188 checks passed
@skjnldsv skjnldsv deleted the printOnlyOnceText branch December 11, 2024 07:36
jancborchardt
jancborchardt reviewed Dec 11, 2024
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Nice update! :) I would also suggest to remove the text "Click the button below to open it" as it’s a duplication of the button right below, and just creates more visual noise.

@solracsf in case you also would like to submit a pull request for that as it improves it even more? :)

@solracsf solracsf mentioned this pull request Dec 13, 2024
Draft
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jancborchardt jancborchardt jancborchardt left review comments

@skjnldsv skjnldsv skjnldsv approved these changes

@artonge artonge artonge approved these changes

@come-nc come-nc Awaiting requested review from come-nc

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish feature: emails
Projects
None yet
Milestone
Nextcloud 31
Development

Successfully merging this pull request may close these issues.
5 participants
@solracsf @skjnldsv @jancborchardt @artonge @come-nc