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

Share link: user mail is disappearing after selecting expiration date #22947

Closed
michaelstingl opened this issue Mar 8, 2016 · 16 comments · Fixed by #23980
Closed

Share link: user mail is disappearing after selecting expiration date #22947

michaelstingl opened this issue Mar 8, 2016 · 16 comments · Fixed by #23980

Comments

@michaelstingl
Copy link

Steps to reproduce

  1. Enable: Allow users to share via link > Allow users to send mail notification for shared files
  2. Share link
  3. Input: Email link to person
  4. Select expiration date with date selector

Expected behaviour

  • Email address shouldn't disappear after setting an expiration data

Actual behaviour

  • Email address disappears after setting an expiration data

2016-03-08 13_20_04

Server configuration

ownCloud version: (see ownCloud admin page)
8.2.2, 8.2.3RC2

Updated from an older ownCloud or fresh install:
fresh install

Client configuration

Browser:
Chrome 49

Operating system:
Mac OS X 10.11.3

@MorrisJobke Who can help here?

00004983

@PVince81
Copy link
Contributor

PVince81 commented Mar 8, 2016

@rullzer

@PVince81 PVince81 added this to the 8.2.4-next-maintenance milestone Mar 8, 2016
@PVince81
Copy link
Contributor

PVince81 commented Mar 8, 2016

Maybe we should move that field to the bottom ? Looks like this user would want to first set all settings before actually sending out the email, which makes sense.

@MorrisJobke
Copy link
Contributor

Maybe we should move that field to the bottom ? Looks like this user would want to first set all settings before actually sending out the email, which makes sense.

@owncloud/designers

@prastut
Copy link
Contributor

prastut commented Mar 8, 2016

I think the set-expiration date should come in the block where all of the settings are present ie password protect, allow editing. @PVince81 makes sense.

I could make a PR for it, let me know. @jancborchardt

@PVince81
Copy link
Contributor

PVince81 commented Mar 8, 2016

The reason why the field clears is because every time a setting is changed, the whole view is re-rendered. Ideally this should be changed to only update the part that was actually modified, not the whole view.

@jancborchardt
Copy link
Member

Yeah, I honestly don’t know why that email field is inbetween those settings. It should be the last one for sure.

Also, it shouldn’t be cleared. Even if the whole view is re-rendered, the value could be remembered and put in again. Or yes, just rerender the part which was modified.

(And repeating myself, ideally it’s combined with the box at the top: #22327 (comment) )

@PVince81
Copy link
Contributor

PVince81 commented Mar 9, 2016

The value isn't part of the share itself, that's why it's not remembered.
But you're right, it could be stored temporarily in a local variable and re-set after rerendering.

@butonic
Copy link
Member

butonic commented Apr 13, 2016

I agree with @jancborchardt the password should be the last input field. That should solve the usability problem.

@PVince81 Can we get the different ordering for 8.2.4?

@MorrisJobke
Copy link
Contributor

@PVince81 Can we get the different ordering for 8.2.4?

He is on vacation. Maybe @rullzer or @ChristophWurst could help out here :)

@ChristophWurst
Copy link
Contributor

@jancborchardt @butonic what should be on the last position? the email field or the password field?

@jancborchardt
Copy link
Member

Email should be last, as @PVince81 and me said above. Not sure if @butonic confused that. ;)

@MorrisJobke
Copy link
Contributor

How critical is this? The fix went into master. It also moves a lot of code and I would vote against a backport to stable8.2. @michaelstingl Is this okay?

@MorrisJobke MorrisJobke modified the milestones: 9.1-current, 8.2.4-current-maintenance Apr 20, 2016
@MorrisJobke
Copy link
Contributor

How critical is this? The fix went into master. It also moves a lot of code and I would vote against a backport to stable8.2. @michaelstingl Is this okay?

cc @karlitschek

@MorrisJobke
Copy link
Contributor

How critical is this? The fix went into master. It also moves a lot of code and I would vote against a backport to stable8.2. @michaelstingl Is this okay?

It seems the backport is already decided: #23980 (comment) ...then nevermind my comment.

@karlitschek
Copy link
Contributor

yes. probably not critical but definitely annoying.

@lock
Copy link

lock bot commented Aug 4, 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 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants