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

remember email when setting expiration date #23980

Merged
merged 1 commit into from Apr 20, 2016

Conversation

ChristophWurst
Copy link
Contributor

fixes #22947

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @PVince81, @blizzz and @rullzer to be potential reviewers

@ChristophWurst
Copy link
Contributor Author

@jancborchardt @butonic if the order is changed so the email field is shown as last element here, the expiration checkbox is shown above the 'share link' checkbox:
bildschirmfoto von 2016-04-14 08-59-46

That makes the usability even worse, because you don't have the option to set an expiration date if it's not a link share…
I guess we have to restructure those views and make the expiration stuff a sub-view of the link sharing view.

@jancborchardt
Copy link
Member

I guess we have to restructure those views and make the expiration stuff a sub-view of the link sharing view.

For sure … the order should be:

  • Share with others
  • Share link
    • Password protect
    • Set expiration date
    • Email link

@ChristophWurst ChristophWurst force-pushed the remember-email-when-setting-expirationdate branch from b727e40 to ab2fab3 Compare April 14, 2016 14:16
@ChristophWurst
Copy link
Contributor Author

@jancborchardt this is what it looks like now (I updated the commit):
bildschirmfoto von 2016-04-14 16-17-24

I'm not sure if it's clear what the different input fields are used for. Should we add some separation between the share information and the email form?

@jancborchardt
Copy link
Member

Maybe it should be a button »Send link via email« by default. On clicking, the input field appears, with an icon-confirm button on the right (like the integrated button on the log in page, not a separate line). What do you think?

@ChristophWurst ChristophWurst force-pushed the remember-email-when-setting-expirationdate branch from ab2fab3 to e7f07ba Compare April 19, 2016 09:36
@PVince81
Copy link
Contributor

Let's make the label say "Send link via email" for now for a backportable solution.

shareAllowed: true,
mailPublicNotificationEnabled: isLinkShare && this.configModel.isMailPublicNotificationEnabled(),
mailPrivatePlaceholder: t('core', 'Email link to person'),
mailButtonText: t('core', 'Send link via email'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PVince81 It already says 'Send link via email' ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah nice... was looking at the old screenshot

@PVince81
Copy link
Contributor

Tested, works 👍

@karlitschek backport for v9.0.2 ? (blue)

@PVince81
Copy link
Contributor

@karlitschek also v8.2.4 apparently

@PVince81
Copy link
Contributor

Needs a second reviewer @rullzer @jancborchardt

@karlitschek
Copy link
Contributor

great. please backport :+1

@jancborchardt
Copy link
Member

Nice

@ChristophWurst
Copy link
Contributor Author

Backport 8.2: #24152
Backport 9: #24153

Note that I backported the simple fix for the actual issue, not the refactoring of the views which might break stuff.

@lock
Copy link

lock bot commented Aug 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Share link: user mail is disappearing after selecting expiration date
6 participants