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.

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

feat(sharebymail): improve share email format #46985

Merged
merged 3 commits into from
Aug 6, 2024
Merged

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Aug 2, 2024

Before After
image image

As discussed:

  1. Removed » and «
  2. Add expiration
  3. Improve layout

Fix #15680
cc @jancborchardt @nimishavijay @marcoambrosini

@skjnldsv skjnldsv added this to the Nextcloud 30 milestone Aug 2, 2024
@skjnldsv skjnldsv self-assigned this Aug 2, 2024
@skjnldsv skjnldsv requested review from nfebe, szaimen and sorbaugh and removed request for a team August 2, 2024 13:30
@skjnldsv skjnldsv force-pushed the feat/email-share-format branch 2 times, most recently from e750265 to 96b2d57 Compare August 2, 2024 14:22
@szaimen szaimen removed their request for review August 2, 2024 14:25
@szaimen
Copy link
Contributor

szaimen commented Aug 2, 2024

I just looked at the after screenshot and I was wondering if technically these elements could be bold?
image

Also, the button on the bottom should probably have the same border-radius that we are useing in Nextcloud for buttons?

cc @nextcloud/designers

@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 2, 2024

I just looked at the after screenshot and I was wondering if technically these elements could be bold?

  • For Note and Expiration, the place to fix it is here. This will change for all apps using our email templating (like calendar events)
    image
  • For the folder title, that would be nice, but it's not supported for now
    We don't support custom html. We could add some custom support like ** to convert to bold during the template processing.
    But I'd say this is something for 31 or later

@skjnldsv

This comment was marked as resolved.

@szaimen

This comment was marked as resolved.

@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 3, 2024

Addressed the border radius, see first post

Two-button example:
image

@Altahrim Altahrim mentioned this pull request Aug 5, 2024
Copy link
Member

@nimishavijay nimishavijay left a comment

Choose a reason for hiding this comment

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

Super nice! Very cool that the buttons are rounded now :)

All looks good, only small change I would suggest is to remove the message in gray at the bottom "X shared Y with you. Click the button below to open it." as it seems redundant now :)

Approving to not block

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the feat/email-share-format branch from eb5bea7 to 879d248 Compare August 6, 2024 07:42
@sorbaugh sorbaugh requested review from susnux and Pytal August 6, 2024 14:35
@skjnldsv skjnldsv merged commit e6457aa into master Aug 6, 2024
167 of 169 checks passed
@skjnldsv skjnldsv deleted the feat/email-share-format branch August 6, 2024 15:40
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 6, 2024
@Altahrim Altahrim mentioned this pull request Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish feature: emails feature: sharing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add expiration date on sharing notification mail
5 participants