feat(core): Minor UI fixes for mobile; autofocus #1499

hyperreality · 2016-09-10T03:49:28Z

feat(core): Minor UI fixes for mobile; autofocus

Social login buttons now 80% of old size, they are too large on small screens such as the iPhone and oversized on the desktop too IMO
Moved the 'sign in failed' warning above the login form, so that it is not cut off on small devices
Fix broken password popover balloon
Added an autofocus directive to automatically move focus to relevant fields on MEAN.js forms. The directive only fires if screen width >= 800px because otherwise a large keyboard may open on a mobile device.
The autofocus directive overwrites the 'autofocus' HTML5 attribute (the attribute is no good for angular apps as it doesn't work across state changes) - I don't believe there's any problem with doing that but I'm open to being corrected

coveralls · 2016-09-10T04:05:37Z

Coverage decreased (-0.2%) to 72.809% when pulling 6a5303c86866de3c178cdb83a14578287fbf2d60 on hyperreality:social-login2 into 17772fe on meanjs:master.

coveralls · 2016-09-10T04:05:37Z

Coverage decreased (-0.2%) to 72.809% when pulling 6a5303c86866de3c178cdb83a14578287fbf2d60 on hyperreality:social-login2 into 17772fe on meanjs:master.

coveralls · 2016-09-10T04:05:37Z

Coverage decreased (-0.2%) to 72.809% when pulling 6a5303c86866de3c178cdb83a14578287fbf2d60 on hyperreality:social-login2 into 17772fe on meanjs:master.

coveralls · 2016-09-10T04:17:11Z

Coverage decreased (-0.2%) to 72.809% when pulling 249fe4e207671384086f332d463ad0279643f3af on hyperreality:social-login2 into 17772fe on meanjs:master.

coveralls · 2016-09-10T04:17:11Z

Coverage decreased (-0.2%) to 72.809% when pulling 249fe4e207671384086f332d463ad0279643f3af on hyperreality:social-login2 into 17772fe on meanjs:master.

coveralls · 2016-09-10T05:09:06Z

Coverage decreased (-0.2%) to 72.809% when pulling 5fe26403b44745dcfe1d0d77d8c43cc2f4c69875 on hyperreality:social-login2 into 17772fe on meanjs:master.

coveralls · 2016-09-10T05:26:13Z

Coverage decreased (-0.2%) to 72.809% when pulling 5fe26403b44745dcfe1d0d77d8c43cc2f4c69875 on hyperreality:social-login2 into 17772fe on meanjs:master.

mleanos · 2016-09-10T17:38:15Z

@hyperreality Can you think of a test to write for this new directive? The coverage decreased, so that may be why the coveralls build failed. I think we have a threshold of coverage decreasing.

@lirantal I couldn't find the settings on Coveralls. Do you know what this setting is, and where I can see it?

hyperreality · 2016-09-10T18:07:04Z

@mleanos I'll play around with sending keypresses on Protractor.

The coveralls settings are a tiny button on the top left.

coveralls · 2016-09-11T04:54:31Z

Coverage decreased (-0.2%) to 72.809% when pulling bdbc3fec01defb7d378987584be328e4ee0ba950 on hyperreality:social-login2 into 17772fe on meanjs:master.

coveralls · 2016-09-11T04:54:31Z

Coverage decreased (-0.2%) to 72.809% when pulling bdbc3fec01defb7d378987584be328e4ee0ba950 on hyperreality:social-login2 into 17772fe on meanjs:master.

hyperreality · 2016-09-11T05:22:36Z

The test is failing on Travis - I suspect that's because the directive only fires if window width is over 800px and Travis ones are smaller than that? However, trying to get Protractor tests working with different windows sizes was a nightmare I don't fancy revisiting for the time being

coveralls · 2016-09-11T05:28:38Z

Coverage decreased (-0.2%) to 72.809% when pulling 98871623645fcc9f6a1a4359ab3cd299cb6bd2cc on hyperreality:social-login2 into b2a5cb5 on meanjs:master.

mleanos · 2016-09-11T18:39:58Z

@hyperreality Ok. Understood. Perhaps we can just leave that test out then?

I'm surprised the coverage decreased so much by merely adding this one directive.

hyperreality · 2016-09-12T10:59:58Z

I'll have one more crack at the test I think.

In the modules directory:
find . -name '*.js' | xargs wc -l reports ~10000 lines of code
wc -l core/client/directives/autofocus.js is 28 lines
Which is about 0.2%

lirantal · 2016-09-12T20:16:03Z

@hyperreality let's see if we can kick this in before a coming 0.5.0 release

coveralls · 2016-09-15T16:12:36Z

Coverage decreased (-0.1%) to 72.869% when pulling 598711a5c145620c82a9aa7b2f45269ceb06ae2d on hyperreality:social-login2 into b2a5cb5 on meanjs:master.

mleanos

I left a couple of comments, but other than those this looks really good. I took it for a test spin, and realized this branch is a bit behind master. We've merged some changes in recently that fixed a couple issues revolving around the Social Logins; including some UI button issues.

Can you rebase this with master, so that I can do a quick manual run through, when you make the requested changes?

mleanos · 2016-09-17T20:39:02Z

modules/users/client/views/settings/manage-social-accounts.client.view.html

-      <a class="btn btn-danger btn-add-remove-account" ng-click="vm.removeUserSocialAccount(providerName)">
-        <i class="glyphicon glyphicon-trash"></i>
+      <a ng-click="vm.removeUserSocialAccount(providerName)">
+        <img class="social-button" ng-src="/modules/users/client/img/buttons/{{providerName}}.png">


These social provider images are being shown twice. If you see above at line 10, the <img> is present there as well. Remove whichever one we don't need. I'm guessing we want this new one that you added.

mleanos · 2016-09-17T20:43:08Z

modules/users/client/views/settings/manage-social-accounts.client.view.html

-        <i class="glyphicon glyphicon-trash"></i>
+      <a ng-click="vm.removeUserSocialAccount(providerName)">
+        <img class="social-button" ng-src="/modules/users/client/img/buttons/{{providerName}}.png">
+        <span class="btn btn-success btn-add-remove-account">


This should be using the btn-danger class, rather than btn-success.

mleanos · 2016-09-17T20:54:06Z

@hyperreality I left a review, but for some reason it doesn't mark the PR as "updated".

simison · 2016-09-17T21:08:26Z

modules/users/client/views/settings/manage-social-accounts.client.view.html

@@ -8,8 +8,11 @@ <h3 class="col-md-12 text-center" ng-show="vm.hasConnectedAdditionalSocialAccoun
    </div>
    <div ng-repeat="(providerName, providerData) in vm.user.additionalProvidersData" class="social-account-container">
      <img ng-src="/modules/users/client/img/buttons/{{providerName}}.png">


It would probably make sense to use one time binding here: {{::providerName}}

Good call, there's several other places in the project where it would make sense too that we'll have to get round to eventually.

coveralls · 2016-09-17T22:16:58Z

Coverage decreased (-0.3%) to 72.932% when pulling 10d822b9153a06fc996cd7c3353164492e272d93 on hyperreality:social-login2 into fa13804 on meanjs:master.

coveralls · 2016-09-17T22:33:25Z

Coverage decreased (-0.3%) to 72.932% when pulling bdb5b9d8d008e65ad0901cf23d1fe2215e5dc22c on hyperreality:social-login2 into fa13804 on meanjs:master.

Fixes css lintings warnings of properties not alphabetized.

Uses the passport info object to simplify login and remove the need to temporarily cache the redirect within the session.

…he server configs. Also added a time indicator on failed login attempts to give the user feedback on subsequent failed login attempts.

…down to the client. reverted some of the other changes (regarding the http request).

…g configs.

Modified config to be "shared". This will allow future configurations to be easily passed to the client.

For New Article, delete function no required

Replaces the $http service calls with promise based methods of the client-side UsersService for the following: Users Change Password Users Manage Social Accounts Users Password Forgot Users Password Reset Users Signup Users Signin Modifies tests to reflect changes. Closes meanjs#1479

coveralls · 2016-09-17T23:04:28Z

Coverage decreased (-0.2%) to 73.058% when pulling 33fb7d1 on hyperreality:social-login2 into fa13804 on meanjs:master.

mleanos · 2016-10-01T21:02:15Z

@hyperreality I'd like to merge this in, but I'm having a hard time reviewing this with all the other commits from your rebase included. I don't know which commits are specific to this PR. Would it be possible for you to clean this up?

mleanos · 2016-10-05T04:14:38Z

@meanjs/contributors @hyperreality

I pulled this branch down locally, and was able to review. Everything seems to be working. I'm merging this now since it introduces a few important fixes, and we need to finalize the other open PR's to get ready for 0.5.0.

hyperreality force-pushed the social-login2 branch 2 times, most recently from 39aa6ae to 5fe2640 Compare September 10, 2016 04:56

lirantal mentioned this pull request Sep 10, 2016

Haven't debug yet, however social login buttons don't work on mobile browers. #1492

Closed

hyperreality force-pushed the social-login2 branch from bdbc3fe to 9887162 Compare September 11, 2016 05:11

mleanos added enhance:feature need:review+1 platform:angular labels Sep 11, 2016

mleanos added this to the 0.5.0 milestone Sep 11, 2016

mleanos self-assigned this Sep 11, 2016

mleanos suggested changes Sep 17, 2016

View reviewed changes

simison reviewed Sep 17, 2016

View reviewed changes

hyperreality force-pushed the social-login2 branch from 598711a to 10d822b Compare September 17, 2016 21:59

hyperreality force-pushed the social-login2 branch from 10d822b to bdb5b9d Compare September 17, 2016 22:13

lirantal and others added 18 commits September 17, 2016 18:45

chore(tidy): tidying up minor lint and layout issues

ccbd025

fix(lint): CSS alphabetize warnings (meanjs#1498)

b0650fe

Fixes css lintings warnings of properties not alphabetized.

fix(authentication) Stops error on signin/signup (meanjs#1495)

b77508f

Uses the passport info object to simplify login and remove the need to temporarily cache the redirect within the session.

Added configuration for owasp. Synchronize client owap configs with t…

b3409c3

…he server configs. Also added a time indicator on failed login attempts to give the user feedback on subsequent failed login attempts.

switched to handlebar template for passing the server's owasp config …

970919c

…down to the client. reverted some of the other changes (regarding the http request).

Removed debug code.

048725d

Changed variable name to owaspConfig

ef78c95

Fixed minor type-o's and set owasp.config() rather than the underlyin…

7a1004f

…g configs.

Moved owasp config into default and reverted other config files.

b5a2eac

Modified config to be "shared". This will allow future configurations to be easily passed to the client.

fixed 403 redirect if not signed in (meanjs#1496)

66ea2f5

Update form-article.client.view.html

b8f403d

For New Article, delete function no required

feat(core): Move template to .github folder

c5fdebf

UI changes for mobile; autofocus

9885975

fixed broken password popover balloon

efa3cf5

add e2e test for autofocus

a143313

Remove test, fix delete social login button

4a90f53

rebase

33fb7d1

hyperreality force-pushed the social-login2 branch from bdb5b9d to 33fb7d1 Compare September 17, 2016 22:46

mleanos approved these changes Oct 5, 2016

View reviewed changes

mleanos merged commit 73a7c2c into meanjs:master Oct 5, 2016

mleanos mentioned this pull request Oct 5, 2016

feat(core): add notification feedback with angular-ui-notification #1532

Merged

hyperreality deleted the social-login2 branch November 20, 2016 02:17

hyperreality mentioned this pull request Dec 3, 2016

feat(core): Better autofocus #1664

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): Minor UI fixes for mobile; autofocus #1499

feat(core): Minor UI fixes for mobile; autofocus #1499

hyperreality commented Sep 10, 2016 •

edited

Loading

coveralls commented Sep 10, 2016 •

edited

Loading

coveralls commented Sep 10, 2016

coveralls commented Sep 10, 2016

coveralls commented Sep 10, 2016 •

edited

Loading

coveralls commented Sep 10, 2016

coveralls commented Sep 10, 2016 •

edited

Loading

coveralls commented Sep 10, 2016 •

edited

Loading

mleanos commented Sep 10, 2016

hyperreality commented Sep 10, 2016 •

edited

Loading

coveralls commented Sep 11, 2016 •

edited

Loading

coveralls commented Sep 11, 2016

hyperreality commented Sep 11, 2016 •

edited

Loading

coveralls commented Sep 11, 2016 •

edited

Loading

mleanos commented Sep 11, 2016

hyperreality commented Sep 12, 2016 •

edited

Loading

lirantal commented Sep 12, 2016

coveralls commented Sep 15, 2016 •

edited

Loading

mleanos left a comment

mleanos Sep 17, 2016

mleanos Sep 17, 2016

hyperreality Sep 17, 2016

mleanos commented Sep 17, 2016

simison Sep 17, 2016 •

edited

Loading

hyperreality Sep 17, 2016

coveralls commented Sep 17, 2016 •

edited

Loading

coveralls commented Sep 17, 2016 •

edited

Loading

coveralls commented Sep 17, 2016 •

edited

Loading

mleanos commented Oct 1, 2016

mleanos commented Oct 5, 2016

feat(core): Minor UI fixes for mobile; autofocus #1499

feat(core): Minor UI fixes for mobile; autofocus #1499

Conversation

hyperreality commented Sep 10, 2016 • edited Loading

coveralls commented Sep 10, 2016 • edited Loading

coveralls commented Sep 10, 2016

coveralls commented Sep 10, 2016

coveralls commented Sep 10, 2016 • edited Loading

coveralls commented Sep 10, 2016

coveralls commented Sep 10, 2016 • edited Loading

coveralls commented Sep 10, 2016 • edited Loading

mleanos commented Sep 10, 2016

hyperreality commented Sep 10, 2016 • edited Loading

coveralls commented Sep 11, 2016 • edited Loading

coveralls commented Sep 11, 2016

hyperreality commented Sep 11, 2016 • edited Loading

coveralls commented Sep 11, 2016 • edited Loading

mleanos commented Sep 11, 2016

hyperreality commented Sep 12, 2016 • edited Loading

lirantal commented Sep 12, 2016

coveralls commented Sep 15, 2016 • edited Loading

mleanos left a comment

Choose a reason for hiding this comment

mleanos Sep 17, 2016

Choose a reason for hiding this comment

mleanos Sep 17, 2016

Choose a reason for hiding this comment

hyperreality Sep 17, 2016

Choose a reason for hiding this comment

mleanos commented Sep 17, 2016

simison Sep 17, 2016 • edited Loading

Choose a reason for hiding this comment

hyperreality Sep 17, 2016

Choose a reason for hiding this comment

coveralls commented Sep 17, 2016 • edited Loading

coveralls commented Sep 17, 2016 • edited Loading

coveralls commented Sep 17, 2016 • edited Loading

mleanos commented Oct 1, 2016

mleanos commented Oct 5, 2016

hyperreality commented Sep 10, 2016 •

edited

Loading

coveralls commented Sep 10, 2016 •

edited

Loading

coveralls commented Sep 10, 2016 •

edited

Loading

coveralls commented Sep 10, 2016 •

edited

Loading

coveralls commented Sep 10, 2016 •

edited

Loading

hyperreality commented Sep 10, 2016 •

edited

Loading

coveralls commented Sep 11, 2016 •

edited

Loading

hyperreality commented Sep 11, 2016 •

edited

Loading

coveralls commented Sep 11, 2016 •

edited

Loading

hyperreality commented Sep 12, 2016 •

edited

Loading

coveralls commented Sep 15, 2016 •

edited

Loading

simison Sep 17, 2016 •

edited

Loading

coveralls commented Sep 17, 2016 •

edited

Loading

coveralls commented Sep 17, 2016 •

edited

Loading

coveralls commented Sep 17, 2016 •

edited

Loading