Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

fix(users): Missing primary social provider #1241

Merged
merged 1 commit into from
Mar 5, 2016

Conversation

mleanos
Copy link
Member

@mleanos mleanos commented Feb 29, 2016

Adds the User's provider to the list of connect social accounts, when it is also a social account.

This is the first step in solving the issue described in #1032. This at least fixes the bug, of the User's provider not showing up.

We could either merge this to satisfy the bug, and then proceed with the rest of the implementation discussed in the issue. Or we could add it to this PR. We'd still need to discuss, and come to a conclusion of how to handle the different scenarios that end User's can face.

@mleanos mleanos added this to the 0.5.0 milestone Feb 29, 2016
@mleanos mleanos self-assigned this Feb 29, 2016
@codydaig
Copy link
Member

codydaig commented Mar 1, 2016

I haven't tested but LGTM.

@@ -1,6 +1,11 @@
<section class="row" ng-controller="SocialAccountsController">
<h3 class="col-md-12 text-center" ng-show="hasConnectedAdditionalSocialAccounts()">Connected social accounts:</h3>
<div class="col-md-12 text-center">
<!-- If the user's provider field (primary) is a social account, show it here //-->
Copy link
Member

Choose a reason for hiding this comment

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

end //?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just a habit. I can remove it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I remove the whole comment?

Copy link
Member

Choose a reason for hiding this comment

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

No just the //, the comment is fine

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. Will do. I think the habit stems from the good ole XML days :)

Adds the User's provider to the list of connect social accounts, when it
is also a social account.

Fixes: meanjs#1032
@mleanos mleanos force-pushed the fix/connected-accounts-not-shown branch from 40a1735 to f2e18e2 Compare March 2, 2016 18:42
@mleanos
Copy link
Member Author

mleanos commented Mar 2, 2016

@ilanbiala I've removed the // from the inline comment.

I'm not really a UI guy, so I'm not sure if there's a better way to indicate that a Social provider is the User's Primary (provider field) account. Any objections to this?
gh-1241

@ilanbiala
Copy link
Member

No, I think that's fine. We're not really building a UI framework anyway, it's just to have some sort of frontend.

@mleanos
Copy link
Member Author

mleanos commented Mar 5, 2016

Merging.. We can discuss the other features that should go into Manage Social Accounts, and handle those enhancements in a new PR.

mleanos added a commit that referenced this pull request Mar 5, 2016
fix(users): Missing primary social provider
@mleanos mleanos merged commit 8f00edc into meanjs:master Mar 5, 2016
@mleanos mleanos deleted the fix/connected-accounts-not-shown branch March 5, 2016 22:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants