Skip to content

Move login page more to the top - #6736

Merged
juliusknorr merged 1 commit into
masterfrom
Dennis1993-patch-1
Nov 9, 2017
Merged

Move login page more to the top#6736
juliusknorr merged 1 commit into
masterfrom
Dennis1993-patch-1

Conversation

@Dennis1993

@Dennis1993Dennis1993 commented Oct 3, 2017•
edited
Loading

Copy link
Copy Markdown
Contributor

on iPhone SE the login button is not shown when the AppInfo is displayed. This change move the page more to the top. The login button is now displayed on small cellphones like iPhone SE

Before:
before

After:
after

@rullzer

rullzer commented Oct 3, 2017

Copy link
Copy Markdown
Member

@nextcloud/designers

@enoch85

enoch85 commented Oct 3, 2017

Copy link
Copy Markdown
Member

OK on mobile devices, not otherwise IMHO

@jancborchardt

jancborchardt commented Oct 3, 2017

Copy link
Copy Markdown
Member

Not sure about this – normally you click away the app installation prompt and then it’s fine.

What do you think @nextcloud/designers?

@Dennis1993

Dennis1993 commented Oct 3, 2017

Copy link
Copy Markdown
ContributorAuthor

If I click it away then it is okay, yes but all elements are not centered. The complete form is on the bottom of the browser on small screens and above the logo is free space in mobile view.

@MariusBluem

MariusBluem commented Oct 3, 2017

Copy link
Copy Markdown
Member

This is only a problem on iPhone SE (and other 4 inch iOS devices), right?

@jancborchardt

jancborchardt commented Oct 4, 2017

Copy link
Copy Markdown
Member

In any case, the way it should be fixed is only on mobile, and only if it’s actually necessary. :) Cutting it all from 100 to 30px is going to make it look bad on other devices.

@skjnldsv

skjnldsv commented Oct 4, 2017

Copy link
Copy Markdown
Member

Why isn't this div directly centered in the middle of the page?

@Dennis1993

Dennis1993 commented Oct 8, 2017•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@MariusBluem Yes, it's only on small devices. I think on large phones the page is more centered.
@jancborchardt Yes! We should fix that only in mobile versions. How can I set my fix only for the mobile version?
Maybe we can create a solution how @skjnldsv discribe it and Center the div in vertical-align? :)

In TOTP the same Problem. Here we can't see the cancel button. The footer is not displayed too
img_3627

@Espina2

Espina2 commented Oct 8, 2017

Copy link
Copy Markdown
Contributor

I made the change for center the div every new release. It what makes sense, I agree with @skjnldsv. If we center it will be fixed in all devices.

@jancborchardt

jancborchardt commented Oct 11, 2017

Copy link
Copy Markdown
Member

I made the change for center the div every new release.

@Espina2 mind submitting a pull request to fix it in the main software? ;)

@Espina2

Espina2 commented Nov 2, 2017

Copy link
Copy Markdown
Contributor

@jancborchardt I can do that.

@jancborchardt

jancborchardt commented Nov 2, 2017

Copy link
Copy Markdown
Member

@Espina2 cool! Tomorrow is feature freeze for Nextcloud 13 just for info, if you want to get it into that. :)

@codecov

codecovBot commented Nov 6, 2017•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #6736 into master will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## master #6736 +/- ##
=========================================
Coverage 50.73% 50.73% Complexity 24413 24413 =========================================
Files 1579 1579 Lines 93294 93294 Branches 1359 1359 =========================================
Hits 47332 47332 Misses 45962 45962

@Espina2

Espina2 commented Nov 6, 2017•
edited
Loading

Copy link
Copy Markdown
Contributor

@jancborchardt @nextcloud/designers Please see if that is okay.

@Espina2

Espina2 commented Nov 6, 2017

Copy link
Copy Markdown
Contributor

Screenshots
screen shot 2017-11-06 at 10 32 41
screen shot 2017-11-06 at 10 32 28

@MorrisJobkeMorrisJobke added 3. to review Waiting for reviews design Design, UI, UX, etc. labels Nov 7, 2017
@Espina2@MorrisJobke
Properly center login page
37c60c2
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke

MorrisJobke commented Nov 7, 2017

Copy link
Copy Markdown
Member

I fixed the code style and rebased on master.

MorrisJobke
MorrisJobke approved these changes Nov 7, 2017
View reviewed changes

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and works 👍

@MorrisJobkeMorrisJobke added this to the Nextcloud 13 milestone Nov 7, 2017
@juliusknorr
juliusknorr merged commit 7dc9f6f into masterNov 9, 2017
@juliusknorr
juliusknorr deleted the Dennis1993-patch-1 branch November 9, 2017 10:10
@Espina2

Espina2 commented Nov 9, 2017

Copy link
Copy Markdown
Contributor

@MorrisJobke Sorry I make the changes directly on github.

@MorrisJobke

MorrisJobke commented Nov 9, 2017

Copy link
Copy Markdown
Member

@MorrisJobke Sorry I make the changes directly on github.

Don't worry :)

@jancborchardt

jancborchardt commented Nov 10, 2017

Copy link
Copy Markdown
Member

Good stuff, thanks @Espina2! :)

@skjnldsv

skjnldsv commented Nov 10, 2017

Copy link
Copy Markdown
Member

Nice! That's what we needed :)

@ChristophWurst

ChristophWurst commented Nov 13, 2017

Copy link
Copy Markdown
Member

⚠️ This caused a regression for U2F and upgrade pages

bildschirmfoto von 2017-11-13 15-58-12
bildschirmfoto von 2017-11-13 15-45-26

@MorrisJobke

MorrisJobke commented Nov 13, 2017

Copy link
Copy Markdown
Member

⚠️ This caused a regression for U2F and upgrade pages

Could you open a new ticket for that?

@ChristophWurstChristophWurst mentioned this pull request Nov 13, 2017
Closed
@jancborchardtjancborchardt mentioned this pull request Nov 23, 2017
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobkeMorrisJobkeMorrisJobke approved these changes
@juliusknorrjuliusknorrjuliusknorr approved these changes
@skjnldsvskjnldsvAwaiting requested review from skjnldsv
@rullzerrullzerAwaiting requested review from rullzer
@nickvergessennickvergessenAwaiting requested review from nickvergessen
@jancborchardtjancborchardtAwaiting requested review from jancborchardt
@raghunayyarraghunayyarAwaiting requested review from raghunayyar

Assignees

No one assigned

Labels

3. to reviewWaiting for reviewsdesignDesign, UI, UX, etc.

Projects

None yet

Milestone

Nextcloud 13

Development

Successfully merging this pull request may close these issues.

10 participants

@Dennis1993@rullzer@enoch85@jancborchardt@MariusBluem@skjnldsv@Espina2@MorrisJobke@ChristophWurst@juliusknorr