UX: Move editing group from into an individual tab.

app/assets/javascripts/admin/controllers/admin-group.js.es6

@@ -1,5 +1,4 @@
 import { popupAjaxError } from 'discourse/lib/ajax-error';
-import { propertyEqual } from 'discourse/lib/computed';
 import computed from 'ember-addons/ember-computed-decorators';
 
 export default Ember.Controller.extend({
@@ -7,19 +6,6 @@ export default Ember.Controller.extend({
   disableSave: false,
   savingStatus: '',
 
-  currentPage: function() {
-    if (this.get("model.user_count") === 0) { return 0; }
-    return Math.floor(this.get("model.offset") / this.get("model.limit")) + 1;
-  }.property("model.limit", "model.offset", "model.user_count"),
-
-  totalPages: function() {
-    if (this.get("model.user_count") === 0) { return 0; }
-    return Math.floor(this.get("model.user_count") / this.get("model.limit")) + 1;
-  }.property("model.limit", "model.user_count"),
-
-  showingFirst: Em.computed.lte("currentPage", 1),
-  showingLast: propertyEqual("currentPage", "totalPages"),
-
   aliasLevelOptions: function() {
     return [
       { name: I18n.t("groups.alias_levels.nobody"), value: 0 },
@@ -47,38 +33,6 @@ export default Ember.Controller.extend({
   },
 
   actions: {
-    next() {
-      if (this.get("showingLast")) { return; }
-
-      const group = this.get("model"),
-            offset = Math.min(group.get("offset") + group.get("limit"), group.get("user_count"));
-
-      group.set("offset", offset);
-
-      return group.findMembers();
-    },
-
-    previous() {
-      if (this.get("showingFirst")) { return; }
-
-      const group = this.get("model"),
-            offset = Math.max(group.get("offset") - group.get("limit"), 0);
-
-      group.set("offset", offset);
-
-      return group.findMembers();
-    },
-
-    removeMember(member) {
-      const self = this,
-            message = I18n.t("admin.groups.delete_member_confirm", { username: member.get("username"), group: this.get("model.name") });
-      return bootbox.confirm(message, I18n.t("no_value"), I18n.t("yes_value"), function(confirm) {
-        if (confirm) {
-          self.get("model").removeMember(member);
-        }
-      });
-    },
-
     removeOwner(member) {
       const self = this,
             message = I18n.t("admin.groups.delete_owner_confirm", { username: member.get("username"), group: this.get("model.name") });
@@ -95,12 +49,6 @@ export default Ember.Controller.extend({
       this.set("model.ownerUsernames", null);
     },
 
-    addMembers() {
-      if (Em.isEmpty(this.get("model.usernames"))) { return; }
-      this.get("model").addMembers(this.get("model.usernames")).catch(popupAjaxError);
-      this.set("model.usernames", null);
-    },
-
     save() {
       const group = this.get('model'),
             groupsController = this.get("adminGroupsType"),

app/assets/javascripts/admin/templates/group.hbs

@@ -33,26 +33,8 @@
       </div>
     {{/unless}}
     <div>
-      <label>{{i18n 'admin.groups.group_members'}} ({{model.user_count}})</label>
+      {{group-members-input model=model}}
-      <div>
-        <a class="previous {{if showingFirst 'disabled'}}" {{action "previous"}}>{{fa-icon "fast-backward"}}</a>
-        {{currentPage}}/{{totalPages}}
-        <a class="next {{if showingLast 'disabled'}}" {{action "next"}}>{{fa-icon "fast-forward"}}</a>
     </div>
-      <div class="ac-wrap clearfix">
-        {{#each model.members as |member|}}
-          {{group-member member=member automatic=model.automatic removeAction="removeMember"}}
-        {{/each}}
-      </div>
-    </div>
-
-    {{#unless model.automatic}}
-      <div>
-        <label for="user-selector">{{i18n 'admin.groups.add_members'}}</label>
-        {{user-selector usernames=model.usernames placeholderKey="admin.groups.selector_placeholder" id="user-selector"}}
-        {{d-button action="addMembers" class="add" icon="plus" label="admin.groups.add"}}
-      </div>
-    {{/unless}}
   {{/if}}
 
   <div>

app/assets/javascripts/discourse/components/group-members-input.js.es6

@@ -0,0 +1,69 @@
+import computed from 'ember-addons/ember-computed-decorators';
+import { popupAjaxError } from 'discourse/lib/ajax-error';
+import { propertyEqual } from 'discourse/lib/computed';
+
+export default Ember.Component.extend({
+  classNames: ["group-members-input"],
+
+  @computed('model.limit', 'model.offset', 'model.user_count')
+  currentPage(limit, offset, userCount) {
+    if (userCount === 0) { return 0; }
+
+    return Math.floor(offset / limit) + 1;
+  },
+
+  @computed('model.limit', 'model.user_count')
+  totalPages(limit, userCount) {
+    if (userCount === 0) { return 0; }
+    return Math.floor(userCount / limit) + 1;
+  },
+
+  @computed('model.usernames')
+  disableAddButton(usernames) {
+    return !usernames || !(usernames.length > 0);
+  },
+
+  showingFirst: Em.computed.lte("currentPage", 1),
+  showingLast: propertyEqual("currentPage", "totalPages"),
+
+  actions: {
+    next() {
+      if (this.get("showingLast")) { return; }
+
+      const group = this.get("model");
+      const offset = Math.min(group.get("offset") + group.get("limit"), group.get("user_count"));
+      group.set("offset", offset);
+
+      return group.findMembers();
+    },
+
+    previous() {
+      if (this.get("showingFirst")) { return; }
+
+      const group = this.get("model");
+      const offset = Math.max(group.get("offset") - group.get("limit"), 0);
+      group.set("offset", offset);
+
+      return group.findMembers();
+    },
+
+    addMembers() {
+      if (Em.isEmpty(this.get("model.usernames"))) { return; }
+      this.get("model").addMembers(this.get("model.usernames")).catch(popupAjaxError);
+      this.set("model.usernames", null);
+    },
+
+    removeMember(member) {
+      const message = I18n.t("groups.edit.delete_member_confirm",{
+        username: member.get("username"),
+        group: this.get("model.name")
+      });
+
+      return bootbox.confirm(message, I18n.t("no_value"), I18n.t("yes_value"), confirm => {
+        if (confirm) {
+          this.get("model").removeMember(member);
+        }
+      });
+    }
+  }
+});

app/assets/javascripts/discourse/controllers/group-edit.js.es6

@@ -1,16 +1,19 @@
 import { popupAjaxError } from 'discourse/lib/ajax-error';
+import computed from 'ember-addons/ember-computed-decorators';
 
 export default Ember.Controller.extend({
-  saving: false,
+  @computed('saving')
+  savingText(saving) {
+    if (saving !== undefined) {
+      return saving ? I18n.t('saving') : I18n.t('saved');
+    }
+  },
 
   actions: {
     save() {
       this.set('saving', true);
 
-      this.get('model').save().then(() => {
+      this.get('model').save().catch(error => {
-        this.transitionToRoute('group', this.get('model.name'));
-        this.send('closeModal');
-      }).catch(error => {
         popupAjaxError(error);
       }).finally(() => {
         this.set('saving', false);

app/assets/javascripts/discourse/controllers/group-index.js.es6

@@ -10,6 +10,7 @@ export default Ember.Controller.extend({
   limit: null,
   offset: null,
   isOwner: Ember.computed.alias('model.is_group_owner'),
+  showActions: false,
 
   @observes('order', 'desc')
   refreshMembers() {
@@ -28,6 +29,10 @@ export default Ember.Controller.extend({
   },
 
   actions: {
+    toggleActions() {
+      this.toggleProperty("showActions");
+    },
+
     removeMember(user) {
       this.get('model').removeMember(user);
     },

app/assets/javascripts/discourse/controllers/group.js.es6

@@ -21,6 +21,10 @@ export default Ember.Controller.extend({
     Tab.create({ name: 'topics' }),
     Tab.create({ name: 'mentions' }),
     Tab.create({ name: 'messages', requiresMembership: true }),
+    Tab.create({
+      name: 'edit', i18nKey: 'edit.title',
+      requiresMembership: true, requiresGroupAdmin: true
+    }),
     Tab.create({
       name: 'logs', i18nKey: 'logs.title',
       requiresMembership: true, requiresGroupAdmin: true

app/assets/javascripts/discourse/routes/app-route-map.js.es6

@@ -57,6 +57,7 @@ export default function() {
     this.route('mentions');
     this.route('messages');
     this.route('logs');
+    this.route('edit');
   });
 
   // User routes

app/assets/javascripts/discourse/routes/group-edit.js.es6

@@ -0,0 +1,15 @@
+export default Ember.Route.extend({
+  titleToken() {
+    return I18n.t('groups.edit.title');
+  },
+
+  model() {
+    return this.modelFor('group');
+  },
+
+  setupController(controller, model) {
+    this.controllerFor('group-edit').setProperties({ model });
+    this.controllerFor("group").set("showing", 'edit');
+    model.findMembers();
+  }
+});

app/assets/javascripts/discourse/routes/group.js.es6

@@ -17,12 +17,5 @@ export default Discourse.Route.extend({
 
   setupController(controller, model) {
     controller.setProperties({ model, counts: this.get('counts') });
-  },
-
-  actions: {
-    showGroupEditor() {
-      showModal('edit-group');
-      this.controllerFor('edit-group').set('model', this.modelFor('group'));
-    }
   }
 });

app/assets/javascripts/discourse/templates/components/group-members-input.hbs

@@ -0,0 +1,23 @@
+<label>{{i18n 'groups.members'}} ({{model.user_count}})</label>
+<div>
+  <a class="previous {{if showingFirst 'disabled'}}" {{action "previous"}}>{{fa-icon "fast-backward"}}</a>
+  {{currentPage}}/{{totalPages}}
+  <a class="next {{if showingLast 'disabled'}}" {{action "next"}}>{{fa-icon "fast-forward"}}</a>
+</div>
+<div class="ac-wrap clearfix">
+  {{#each model.members as |member|}}
+    {{group-member member=member automatic=model.automatic removeAction="removeMember"}}
+  {{/each}}
+</div>
+
+{{#unless model.automatic}}
+  <div class="group-members-input-selector">
+    {{user-selector usernames=model.usernames}}
+
+    {{d-button action="addMembers"
+        class="add"
+        icon="plus"
+        disabled=disableAddButton
+        label="groups.edit.add_members"}}
+  </div>
+{{/unless}}

app/assets/javascripts/discourse/templates/group-edit.hbs

@@ -0,0 +1,46 @@
+<div class='group-edit'>
+  <form class="form-horizontal">
+    <div class="control-group">
+      <label for='title'>{{i18n 'groups.edit.group_title'}}</label>
+      {{input type='text' name='title' value=model.title class='group-edit-title'}}
+    </div>
+
+    <div class="control-group">
+      <label for='bio'>{{i18n 'groups.bio'}}</label>
+      {{d-editor value=model.bio_raw class="group-edit-bio"}}
+    </div>
+
+    <div class="control-group">
+      {{group-members-input model=model}}
+    </div>
+
+    <div class="control-group">
+      {{group-flair-inputs model=model}}
+    </div>
+
+    <div class="control-group">
+      <label>
+        {{input type='checkbox'
+            checked=model.public
+            class="group-edit-public"
+            disabled=model.allow_membership_requests}}
+
+        {{i18n 'groups.public'}}
+      </label>
+    </div>
+
+    <div class="control-group">
+      <label>
+        {{input type='checkbox'
+            checked=model.allow_membership_requests
+            class="group-edit-allow-membership-requests"
+            disabled=model.public}}
+
+        {{i18n 'groups.allow_membership_requests'}}
+      </label>
+    </div>
+
+    {{d-button action="save" class="btn-primary" disabled=saving label="save"}}
+    {{savingText}}
+  </form>
+</div>

app/assets/javascripts/discourse/templates/group-index.hbs

@@ -1,10 +1,5 @@
 {{#if model}}
-  {{#if isOwner}}
+  {{#if canJoinGroup}}
-    <form id='add-user-to-group' autocomplete="off">
-      {{user-selector usernames=usernames placeholderKey="groups.selector_placeholder" id="user-search-selector" name="usernames"}}
-      {{d-button action="addMembers" class="add" icon="plus" label="groups.add"}}
-    </form>
-  {{else if canJoinGroup}}
     {{#if model.is_group_user}}
       {{d-button action="leaveGroup"
           class="btn-danger group-index-leave"
@@ -31,7 +26,6 @@
         {{group-index-toggle order=order desc=desc field='username_lower' i18nKey='username'}}
         {{group-index-toggle order=order desc=desc field='last_posted_at' i18nKey='last_post'}}
         {{group-index-toggle order=order desc=desc field='last_seen_at'  i18nKey='last_seen'}}
-        <th></th>
       </thead>
 
       <tbody>
@@ -48,13 +42,6 @@
             <td>
               <span class="text">{{bound-date m.last_seen_at}}</span>
             </td>
-            <td class='remove-user'>
-              {{#if isOwner}}
-                {{#unless m.owner}}
-                  <a class="remove-link" {{action "removeMember" m}}><i class="fa fa-times"></i></a>
-                {{/unless}}
-              {{/if}}
-            </td>
           </tr>
         {{/each}}
       </tbody>

app/assets/javascripts/discourse/templates/group.hbs

@@ -20,12 +20,6 @@
           <h3 class='group-name'>@{{model.name}}</h3>
         {{/if}}
       </span>
-
-      {{#if canEditGroup}}
-        <span class="group-edit">
-          {{d-button action="showGroupEditor" label="groups.edit.title" class="group-edit-btn" icon="pencil"}}
-        </span>
-      {{/if}}
     </div>
 
     {{#if model.bio_cooked}}

app/assets/javascripts/discourse/templates/modal/edit-group.hbs

@@ -1,34 +0,0 @@
-{{#d-modal-body title="groups.edit.title" class="edit-group groups"}}
-  <form class="form-horizontal">
-    <label for='title'>{{i18n 'groups.edit.group_title'}}</label>
-    {{input type='text' name='title' value=model.title class='edit-group-title'}}
-
-    <label for='bio'>{{i18n 'groups.bio'}}</label>
-    {{d-editor value=model.bio_raw class="edit-group-bio"}}
-
-    {{group-flair-inputs model=model}}
-
-    <label>
-      {{input type='checkbox'
-          checked=model.public
-          class="edit-group-public"
-          disabled=model.allow_membership_requests}}
-
-      {{i18n 'groups.public'}}
-    </label>
-
-    <label>
-      {{input type='checkbox'
-          checked=model.allow_membership_requests
-          class="edit-group-allow-membership-requests"
-          disabled=model.public}}
-
-      {{i18n 'groups.allow_membership_requests'}}
-    </label>
-  </form>
-{{/d-modal-body}}
-
-<div class="modal-footer">
-  {{d-button action="save" class="btn-primary" disabled=saving label="save"}}
-  <a {{action "closeModal"}}>{{i18n 'cancel'}}</a>
-</div>

app/assets/stylesheets/common/base/group-members-input.scss

@@ -0,0 +1,15 @@
+.group-members-input {
+  .ac-wrap {
+    width: 100% !important;
+  }
+
+  .group-members-inputs-selector {
+    margin-top: 10px;
+
+    .add {
+      margin-top: 7px;
+    }
+  }
+}
+
+

app/assets/stylesheets/common/base/group.scss

@@ -62,23 +62,22 @@ table.group-members {
 
   th:first-child {
     width: 60%;
+    text-align: left;
   }
 
-  th:last-child {
+  th.group-members-actions {
     width: 5%;
   }
 
   th {
     border-bottom: 3px solid dark-light-diff($primary, $secondary, 90%, -60%);
+    text-align: center;
+    padding: 5px 0px 5px 5px;
+    color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));
+    font-weight: normal;
   }
 
   th.sortable {
-    text-align: left;
-    padding: 5px 0px 5px 5px;
-
-    color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%));
-    font-weight: normal;
-
     i {
       margin-left: 5px;
     }
@@ -94,7 +93,12 @@ table.group-members {
       display: block;
     }
 
+    td:first-child {
+      text-align: left;
+    }
+
     td {
+      text-align: center;
       color: dark-light-diff($primary, $secondary, 50%, -50%);
       padding: 0.8em 0;
     }
@@ -128,14 +132,9 @@ table.group-members {
   }
 }
 
-.group-edit {
-  float: right;
-}
-
 .form-horizontal {
   .group-flair-inputs {
     display: inline-block;
-    margin: 15px 0px;
 
     input[type="text"] {
       width: 80% !important;
@@ -162,11 +161,7 @@ table.group-members {
   }
 }
 
-.groups.edit-group .form-horizontal {
+.group-edit .form-horizontal {
-  textarea {
-    width: 99%;
-  }
-
   label {
     font-weight: bold;
   }

app/controllers/groups_controller.rb

@@ -14,6 +14,9 @@ class GroupsController < ApplicationController
     render_serialized(find_group(:id), GroupShowSerializer, root: 'basic_group')
   end
 
+  def edit
+  end
+
   def update
     group = Group.find(params[:id])
     guardian.ensure_can_edit!(group)

config/locales/client.en.yml

@@ -397,6 +397,8 @@ en:
       edit:
         title: 'Edit Group'
         group_title: 'Title'
+        add_members: "Add Members"
+        delete_member_confirm: "Remove '%{username}' from the '%{group}' group?"
       request_membership_pm:
         title: "Membership Request"
         body: "I would like to request membership in @%{groupName}."
@@ -2499,7 +2501,6 @@ en:
         delete: "Delete"
         delete_confirm: "Delete this group?"
         delete_failed: "Unable to delete group. If this is an automatic group, it cannot be destroyed."
-        delete_member_confirm: "Remove '%{username}' from the '%{group}' group?"
         delete_owner_confirm: "Remove owner privilege for '%{username}'?"
         add: "Add"
         add_members: "Add members"

test/javascripts/acceptance/group-edit-test.js.es6

@@ -0,0 +1,33 @@
+import { acceptance } from "helpers/qunit-helpers";
+
+acceptance("Editing Group", {
+  loggedIn: true
+});
+
+test("Editing group", () => {
+  visit("/groups/discourse/edit");
+
+  andThen(() => {
+    ok(find('.group-flair-inputs').length === 1, 'it should display avatar flair inputs');
+    ok(find('.group-edit-bio').length === 1, 'it should display group bio input');
+    ok(find('.group-edit-title').length === 1, 'it should display group title input');
+    ok(find('.group-edit-public').length === 1, 'it should display group public input');
+    ok(find('.group-edit-allow-membership-requests').length === 1, 'it should display group allow_membership_requets input');
+    ok(find('.group-members-input .item').length === 7, 'it should display group members');
+    ok(find('.group-members-input-selector').length === 1, 'it should display input to add group members');
+    ok(find('.group-members-input-selector .add[disabled]').length === 1, 'add members button should be disabled');
+  });
+
+  click('.group-edit-public');
+
+  andThen(() => {
+    ok(find('.group-edit-allow-membership-requests[disabled]').length === 1, 'it should disable group allow_membership_requets input');
+  });
+
+  click('.group-edit-public');
+  click('.group-edit-allow-membership-requests');
+
+  andThen(() => {
+    ok(find('.group-edit-public[disabled]').length === 1, 'it should disable group public input');
+  });
+});

test/javascripts/acceptance/groups-test.js.es6

@@ -27,8 +27,9 @@ test("Browsing Groups", () => {
 
   visit("/groups/discourse/messages");
   andThen(() => {
-    ok(find(".nav-stacked li a[title='Messages']").length === 0, 'it should not show messages tab if user is admin');
+    ok(find(".nav-stacked li a[title='Messages']").length === 0, 'it should not show messages tab if user is not admin');
-    ok(find(".nav-stacked li a[title='Logs']").length === 0, 'it should not show Logs tab if user is admin');
+    ok(find(".nav-stacked li a[title='Edit Group']").length === 0, 'it should not show messages tab if user is not admin');
+    ok(find(".nav-stacked li a[title='Logs']").length === 0, 'it should not show Logs tab if user is not admin');
     ok(count('.user-stream .item') > 0, "it lists stream items");
   });
 });
@@ -41,31 +42,9 @@ test("Admin Browsing Groups", () => {
 
   andThen(() => {
     ok(find(".nav-stacked li a[title='Messages']").length === 1, 'it should show messages tab if user is admin');
+    ok(find(".nav-stacked li a[title='Edit Group']").length === 1, 'it should show edit group tab if user is admin');
     ok(find(".nav-stacked li a[title='Logs']").length === 1, 'it should show Logs tab if user is admin');
     equal(find('.group-title').text(), 'Awesome Team', 'it should display the group title');
     equal(find('.group-name').text(), '@discourse', 'it should display the group name');
   });
-
-  click('.group-edit-btn');
-
-  andThen(() => {
-    ok(find('.group-flair-inputs').length === 1, 'it should display avatar flair inputs');
-    ok(find('.edit-group-bio').length === 1, 'it should display group bio input');
-    ok(find('.edit-group-title').length === 1, 'it should display group title input');
-    ok(find('.edit-group-public').length === 1, 'it should display group public input');
-    ok(find('.edit-group-allow-membership-requests').length === 1, 'it should display group allow_membership_requets input');
-  });
-
-  click('.edit-group-public');
-
-  andThen(() => {
-    ok(find('.edit-group-allow-membership-requests[disabled]').length === 1, 'it should disable group allow_membership_requets input');
-  });
-
-  click('.edit-group-public');
-  click('.edit-group-allow-membership-requests');
-
-  andThen(() => {
-    ok(find('.edit-group-public[disabled]').length === 1, 'it should disable group public input');
-  });
 });