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

Fix #1599 consistency when joining moderated-membership comm #1803

Merged
merged 2 commits into from Jun 18, 2016

Conversation

cesy
Copy link
Contributor

@cesy cesy commented Jun 3, 2016

Contextual hover now shows accept instead of join if you have
a pending invitation. See #1599 for details.

Contextual hover now shows accept instead of join if you have
a pending invitation.
if (data.is_member) {
membershipLink.href = data.url_leavecomm;
membershipLink.innerHTML = "Leave";
var membership_action = "leave"
Copy link
Member

Choose a reason for hiding this comment

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

I do see one possible issue here, which is that you've moved the variable scope of membership_action (the "var" declaration) into the conditional, which may mean (if the rules for JS are the same as for Perl) that the assignment is lost outside of that conditional. I would keep the scope declaration above:

var membership_action;

And then assign to the variable in each conditional without the var qualifier:

membership_action = "leave"
membership_action = "accept"
membership_action = "join"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, fixed.


if (!data.is_closed_membership || data.is_member) {
var membershipLink = document.createElement("a");

var membership_action = data.is_member ? "leave" : "join";
Copy link
Member

Choose a reason for hiding this comment

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

Let's put the var membership_action statement back here where it was previously, just to keep the scope the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, done.

@kareila
Copy link
Member

kareila commented Jun 18, 2016

I did discover one edge case when testing this: if you accept the invitation, then click again to leave the community, it goes back to "Accept Invitation" instead of "Join Community", and if you click a third time, nothing happens. Not sure if it's worth the effort to correct, since it's not likely that someone will want to do that on purpose, but it might get double-clicked accidentally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants