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

Bugfix/change password changes #1229

Merged
merged 6 commits into from
Oct 6, 2016
Merged

Conversation

raghunayyar
Copy link
Member

Fixes all issues stated in
#1061

Please review @nextcloud/designers @jancborchardt

@mention-bot
Copy link

@raghunayyar, thanks for your PR! By analyzing the annotation information on this pull request, we identified @jancborchardt, @MTGap and @Henni to be potential reviewers

@raghunayyar raghunayyar added 3. to review Waiting for reviews enhancement design Design, UI, UX, etc. labels Sep 1, 2016
@raghunayyar
Copy link
Member Author

I see there is an issue with the tipsy positioning in my last commit, fixing that.

@raghunayyar
Copy link
Member Author

Fixed, up for review now. :)

@nickvergessen nickvergessen added this to the Nextcloud 11.0 milestone Sep 1, 2016
@MorrisJobke
Copy link
Member

MorrisJobke commented Sep 1, 2016

  • BUG: enter a wrong password and click "Change password" causes the spinner to stay endless

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Sep 1, 2016
@raghunayyar raghunayyar added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 1, 2016
@raghunayyar
Copy link
Member Author

@MorrisJobke done ;)

@raghunayyar
Copy link
Member Author

@nextcloud/designers need a couple of thumbs up here. ;)

@MorrisJobke
Copy link
Member

  • BUG: enable password policy app and try to change password to abc -> error message is not shown
  • BUG: on clicking "change password" the page jumps down a whole screen and the app password section is shown and focused - looks like duplicate use of an ID

@MorrisJobke
Copy link
Member

@raghunayyar sorry for those new issues :(

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Sep 7, 2016
@MorrisJobke
Copy link
Member

Beside the two points it works very well.

@MorrisJobke
Copy link
Member

BUG: enable password policy app and try to change password to abc -> error message is not shown

I think this one could be done in an additional ticket.

@raghunayyar
Copy link
Member Author

@MorrisJobke I still can't reproduce the bug you are talking about and you showed me in the conference on my machine. Can you take this up if you got some extra time?

@MorrisJobke
Copy link
Member

@MorrisJobke I still can't reproduce the bug you are talking about and you showed me in the conference on my machine. Can you take this up if you got some extra time?

Is this the only thing left? Then change the label to "3 - To review" and I will give this another try ;)

@raghunayyar raghunayyar added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 4, 2016
@raghunayyar
Copy link
Member Author

raghunayyar commented Oct 4, 2016

All yours @MorrisJobke

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Copy link
Member

I removed the old "Saving ..." and "Successfully saved" messages, because they only duplicate the feedback.

Beside that it works nicely 👍

@MorrisJobke
Copy link
Member

@rullzer @nickvergessen Please review

@MorrisJobke MorrisJobke removed their assignment Oct 6, 2016
@MorrisJobke
Copy link
Member

I will fix the issue with the password policy app in a separate PR

@MorrisJobke
Copy link
Member

I will fix the issue with the password policy app in a separate PR

#1634

@raghunayyar
Copy link
Member Author

thanks @MorrisJobke, much appreciated. :D

@raghunayyar
Copy link
Member Author

Also, should this be backported? @karlitschek

@karlitschek
Copy link
Member

nice. please backport 👍

@MorrisJobke
Copy link
Member

Also, should this be backported? @karlitschek

I would not backport this one here. It's mostly cosmetic stuff and it works in stable versions.

@MorrisJobke MorrisJobke merged commit 8fecf85 into master Oct 6, 2016
@MorrisJobke MorrisJobke deleted the bugfix/change-password-changes branch October 6, 2016 10:44
@MorrisJobke
Copy link
Member

I would not backport this one here. It's mostly cosmetic stuff and it works in stable versions.

Is this okay with you, @raghunayyar? @nickvergessen @LukasReschke Opinions?

@jancborchardt
Copy link
Member

It’s polishing and non-critical, so I wouldn’t backport it really.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews design Design, UI, UX, etc. enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants