[fix bug 969039] Change moderated group's join notification. #836

Merged
merged 1 commit into from Mar 19, 2014

Projects

None yet

3 participants

@glogiotatidis
Member

No description provided.

@glogiotatidis
Member
@dpoirier dpoirier and 1 other commented on an outdated diff Mar 4, 2014
mozillians/groups/views.py
@@ -262,8 +262,7 @@ def join_group(request, group_pk):
messages.info(request, _('You have been added to this group.'))
elif group.accepting_new_members == 'by_request':
group.add_member(profile_to_add, status=GroupMembership.PENDING)
- messages.info(request,
- _('Your membership request is pending approval by the group curator.'))
+ messages.info(request, _('Membership request sent to group curator.'))
@dpoirier
dpoirier Mar 4, 2014

I wonder if translators would prefer a little more verbose sentence - e.g. "Your membership request has been sent to the group curator"?

@glogiotatidis
glogiotatidis Mar 4, 2014 Mozilla member

Updated.

@dpoirier dpoirier commented on the diff Mar 4, 2014
mozillians/static/mozillians/css/main.less
@@ -211,16 +211,6 @@ body {
content: " *";
color: @linkRed;
}
-
- .group-pending-membership {
@dpoirier
dpoirier Mar 4, 2014

How about if we continue using the group-pending-membership class name, and if we want to make it look like an alert-warning for now, mix in that style in the .less file? It'd be a nudge toward more semantic markup. @wraybowling what do you think?

@glogiotatidis
glogiotatidis Mar 4, 2014 Mozilla member

I don't have any opinion / knownledge on semantic markup but instead of being a class, can't it just be an id?

@glogiotatidis
Member

ping?

@wraybowling

Hi guys. My most recent research on how to use extensions in LESS could do this better. There are a lot of areas of the CSS that could be done better and this is just one of them. I was previously in the camp that using LESS was a bad idea because the use of mixins has made our stylesheet become really large and ugly, but extensions would do the job and are pretty nice.

@glogiotatidis
Member

I'm not really sure what's suggested here. @dpoirier can you please clarify?

@dpoirier

I just wanted to mention that this might be an opportunity to start using more semantic markup. I leave it to @wraybowling to make a more specific recommendation for this pull request.

@wraybowling

I agree. When I started making these classes, I wasn't aware that LESS was capable of using extensions. Let's talk about doing a stylesheet & semantics cleanup soon. That's something I think I could contribute a lot to.

@glogiotatidis
Member

@wraybowling is there something specific to this pull request to suggest or should we just merge?

@glogiotatidis
Member
<giorgos> dpoirier, morning. I'm merging pr 836, sounds good?
<dpoirier> yes I think go ahead
@glogiotatidis glogiotatidis merged commit 4c136a2 into mozilla:master Mar 19, 2014

1 check passed

Details default Jenkins build 'mozillians_github' #1252 has succeeded
@glogiotatidis glogiotatidis deleted the glogiotatidis:969039-3 branch Jun 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment