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

Login Textfield design #1010

Merged
merged 2 commits into from
Feb 24, 2021
Merged

Login Textfield design #1010

merged 2 commits into from
Feb 24, 2021

Conversation

AndyScherzinger
Copy link
Member

@AndyScherzinger AndyScherzinger commented Feb 15, 2021

Original, corresponding Android Files client PR based on the design review by @jancborchardt and others (nextcloud/android#7971):

  • Using container-style for text fields (initially just for the login screen since it has a specific set of colors)

Screenshots

Filled Address Error Text Checking Server
device-2021-02-15-153947 device-2021-02-15-155501 device-2021-02-15-154229

Sidenote

  • atm the talk client simply states the same error (see screenshot) no matter the error (e.g. server doesn't even exists Vs. talk app not installed). This could be changed but is not related to this UI PR and can be added in the future by anybody, the UI is in pace
  • talk client has a icon opacity logic for enabled/disabled which the files client lacks atm (so either remove or add to the files client in the future)
  • like with the files client the helper-text is brand-able

Other than that

@AndyScherzinger AndyScherzinger added 3. to review Waiting for reviews design Related to the design labels Feb 15, 2021
@tobiasKaminsky
Copy link
Member

* talk client has a icon opacity logic for enabled/disabled which the files client lacks atm (so either remove or add to the files client in the future)

I think this is not needed. if you want, you can of course add it on Files app, but it is not that needed…

@AndyScherzinger
Copy link
Member Author

@jancborchardt I vote for also putting some more spacing between logo and input field, e.g. double it.

@jancborchardt
Copy link
Member

@jancborchardt I vote for also putting some more spacing between logo and input field, e.g. double it.

Yep, good call! Also agree with removing the opacity/transparency logic. More feedback:

  • Can we define a max-width for that whole block of content? It looks off spanning full width, e.g. here half the width of what you have in the screenshots is already plenty. This would probably be defined in dp instead of percentage so it’s the same no matter the display size.
  • The "Testing connection" and "Server does not have supported …" is not shown in the same place, would be nicer if it is. :)

@AndyScherzinger
Copy link
Member Author

Can we define a max-width for that whole block of content? It looks off spanning full width, e.g. here half the width of what you have in the screenshots is already plenty. This would probably be defined in dp instead of percentage so it’s the same no matter the display size.

I'll look into it while I am pretty sure its possible. Would add that for files client too. (Reason you see it here: files client screenshots (phone), talk (tablet) 😉

The "Testing connection" and "Server does not have supported …" is not shown in the same place, would be nicer if it is. :)

I'll look into it, sure 👍

@AndyScherzinger
Copy link
Member Author

@jancborchardt All DONE, updated screenshots #1010 (comment)

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Bellissimo! ✨

@AndyScherzinger
Copy link
Member Author

Mille Gracie 🙏

@tobiasKaminsky
Copy link
Member

For drone to work again, please rebase after #1012 is merged.

@tobiasKaminsky
Copy link
Member

Rebased

Copy link
Collaborator

@mahibi mahibi left a comment

Choose a reason for hiding this comment

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

this looks great, thank you @AndyScherzinger !!

@mahibi
Copy link
Collaborator

mahibi commented Feb 19, 2021

waiting for #1018 (#1014 was for master, this one is for master-broken)

@mahibi mahibi force-pushed the loginTextfieldDesign branch 2 times, most recently from ce5f95f to b3c02c0 Compare February 22, 2021 15:31
@mahibi mahibi force-pushed the loginTextfieldDesign branch 3 times, most recently from 296ed6f to 7aaeea4 Compare February 23, 2021 09:36
…ip ci]

Signed-off-by: drone <noreply@drone>
@nextcloud-android-bot
Copy link
Collaborator

Lint

TypeMasterPR
Warnings566566
Errors1111

FindBugs (new)

Warning Type Number
Bad practice Warnings 8
Correctness Warnings 107
Experimental Warnings 2
Performance Warnings 11
Security Warnings 8
Dodgy code Warnings 98
Total 234

FindBugs (master)

Warning Type Number
Bad practice Warnings 8
Correctness Warnings 107
Experimental Warnings 2
Performance Warnings 11
Security Warnings 8
Dodgy code Warnings 98
Total 234

@mahibi
Copy link
Collaborator

mahibi commented Feb 24, 2021

so... we just don't know why drone doesn't use the correct email address (#1021 (comment))

so PR is finally merged with admin rights.. thanks again @AndyScherzinger

@mahibi mahibi merged commit 12154ff into master-broken Feb 24, 2021
@delete-merged-branch delete-merged-branch bot deleted the loginTextfieldDesign branch February 24, 2021 08:56
@AndyScherzinger
Copy link
Member Author

My pleasure 👍

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 Related to the design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants