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 autofill optimizations #1481

Merged
merged 2 commits into from
Sep 12, 2017
Merged

Login autofill optimizations #1481

merged 2 commits into from
Sep 12, 2017

Conversation

dev7ch
Copy link
Contributor

@dev7ch dev7ch commented Sep 11, 2017

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.619% when pulling 1cb0a48 on dev7ch:login-autofill into 3f5d7de on luyadev:master.

@@ -12,6 +12,8 @@
<?php $this->beginBody() ?>
<?= $content; ?>
<?php $this->endBody() ?>
<? $this->registerJS('$(window).load(function () {checkInputLabels(); });'); ?>
Copy link
Member

Choose a reason for hiding this comment

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

checkInputLabels() is already execute in the login.js script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, but we need to run this function in window on load context.

Copy link
Member

Choose a reason for hiding this comment

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

we did, directly inside the login.js

@nadar nadar merged commit c3f1d54 into luyadev:master Sep 12, 2017
nadar added a commit that referenced this pull request Sep 12, 2017
@nadar
Copy link
Member

nadar commented Sep 12, 2017

@dev7ch i just cleanup your code and the logic inside your javascript changes makes no sense as the element checking behavior does not require an ID

var mail = document.getElementById("login-user-email");
var pass = document.getElementById("login-user-password");

so this makes no sense as val contains the element.

And it still does not work

bildschirmfoto 2017-09-12 um 08 17 05

@dev7ch
Copy link
Contributor Author

dev7ch commented Sep 12, 2017

i am pretty sure ist an compiling issue, after compile i get this as expected:

image

But i get also an Angular Error (white screen) after login (compile main.js breaks sth):

image

Its not related to the login.js, its this issue still persist?

@nadar
Copy link
Member

nadar commented Sep 12, 2017

make sure you have run composer install in the admin and cms modules.

@TheMaaarc
Copy link
Member

webkit-autofill and moz-autofill are pretty buggy. For example chrome handles type text and password differently, that's why the password label only floats to the top if you click anywhere on the page - until you do that the password field is considered empty by chrome.
I suggest we stop with that floating label stuff and just write the label above the input.

@dev7ch
Copy link
Contributor Author

dev7ch commented Sep 12, 2017

@nadar ok thanks, composer update inside admin module fixed copmile issue;

@marc, ok yes, i ve noticed its kind of pretty buggy. I ll keep this in mind for the future but for now i hope the implementation works a bit more reliable then before( testet in Firefox, Chrom, Opera, Safari - latest).

@marc, @nadar is the the floating autofill still not working for you?

@nadar
Copy link
Member

nadar commented Sep 12, 2017

i am not sure, i have to rewrite all your code first.

@dev7ch i just cleanup your code and the logic inside your javascript changes makes no sense as the element checking behavior does not require an ID

var mail = document.getElementById("login-user-email");
var pass = document.getElementById("login-user-password");

so this makes no sense as val contains the element.

TheMaaarc added a commit that referenced this pull request Sep 12, 2017
dev7ch added a commit to dev7ch/luya that referenced this pull request Sep 12, 2017
@dev7ch dev7ch mentioned this pull request Sep 12, 2017
nadar pushed a commit that referenced this pull request Sep 13, 2017
* adding checkInputsLabels as on load func

* move window onload function into js file #1481 fix #1460
@dev7ch dev7ch deleted the login-autofill branch October 28, 2017 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants