UX screen review fixes for Admin Console Teams/Channels (mattermost#3058

)



Co-authored-by: Martin Kraft <[email protected]>

components/add_groups_to_channel_modal/__snapshots__/add_groups_to_channel_modal.test.jsx.snap

@@ -111,10 +111,12 @@ exports[`components/AddGroupsToChannelModal should match when renderOption is ca
  <div
  className="more-modal__name"
  >
-  
+  
  -
- 
- <span>
+  
+ <span
+ className="more-modal__name_sub"
+ >
  <FormattedMessage
  defaultMessage="{num, number} {num, plural, one {member} other {members}}"
  id="numMembers"
@@ -157,10 +159,12 @@ exports[`components/AddGroupsToChannelModal should match when renderOption is ca
  <div
  className="more-modal__name"
  >
-  
+  
  -
- 
- <span>
+  
+ <span
+ className="more-modal__name_sub"
+ >
  <FormattedMessage
  defaultMessage="{num, number} {num, plural, one {member} other {members}}"
  id="numMembers"
@@ -203,10 +207,12 @@ exports[`components/AddGroupsToChannelModal should match when renderOption is ca
  <div
  className="more-modal__name"
  >
-  
+  
  -
- 
- <span>
+  
+ <span
+ className="more-modal__name_sub"
+ >
  <FormattedMessage
  defaultMessage="{num, number} {num, plural, one {member} other {members}}"
  id="numMembers"

components/add_groups_to_channel_modal/add_groups_to_channel_modal.jsx

@@ -189,7 +189,7 @@ export default class AddGroupsToChannelModal extends React.Component {
  className='more-modal__details'
  >
  <div className='more-modal__name'>
- {option.display_name} {'-'} <span>
+ {option.display_name}&nbsp;{'-'}&nbsp;<span className='more-modal__name_sub'>
  <FormattedMessage
  id='numMembers'
  defaultMessage='{num, number} {num, plural, one {member} other {members}}'
@@ -235,10 +235,12 @@ export default class AddGroupsToChannelModal extends React.Component {
  }
  let groupsToShow = this.props.groups;
  if (this.props.excludeGroups) {
- groupsToShow = groupsToShow.filter((g) => !this.props.excludeGroups.includes(g));
+ const hasGroup = (og) => !this.props.excludeGroups.find((g) => g.id === og.id);
+ groupsToShow = groupsToShow.filter(hasGroup);
  }
  if (this.props.includeGroups) {
- groupsToShow = [...groupsToShow, ...this.props.includeGroups.filter((g) => !groupsToShow.includes(g))];
+ const hasGroup = (og) => this.props.includeGroups.find((g) => g.id === og.id);
+ groupsToShow = [...groupsToShow, ...this.props.includeGroups.filter(hasGroup)];
  }
 
  return (

components/add_groups_to_team_modal/__snapshots__/add_groups_to_team_modal.test.jsx.snap

@@ -111,10 +111,12 @@ exports[`components/AddGroupsToTeamModal should match when renderOption is calle
  <div
  className="more-modal__name"
  >
-  
+  
  -
- 
- <span>
+  
+ <span
+ className="more-modal__name_sub"
+ >
  <FormattedMessage
  defaultMessage="{num, number} {num, plural, one {member} other {members}}"
  id="numMembers"
@@ -157,10 +159,12 @@ exports[`components/AddGroupsToTeamModal should match when renderOption is calle
  <div
  className="more-modal__name"
  >
-  
+  
  -
- 
- <span>
+  
+ <span
+ className="more-modal__name_sub"
+ >
  <FormattedMessage
  defaultMessage="{num, number} {num, plural, one {member} other {members}}"
  id="numMembers"
@@ -203,10 +207,12 @@ exports[`components/AddGroupsToTeamModal should match when renderOption is calle
  <div
  className="more-modal__name"
  >
-  
+  
  -
- 
- <span>
+  
+ <span
+ className="more-modal__name_sub"
+ >
  <FormattedMessage
  defaultMessage="{num, number} {num, plural, one {member} other {members}}"
  id="numMembers"

components/add_groups_to_team_modal/add_groups_to_team_modal.jsx

@@ -188,7 +188,7 @@ export default class AddGroupsToTeamModal extends React.Component {
  className='more-modal__details'
  >
  <div className='more-modal__name'>
- {option.display_name} {'-'} <span>
+ {option.display_name}&nbsp;{'-'}&nbsp;<span className='more-modal__name_sub'>
  <FormattedMessage
  id='numMembers'
  defaultMessage='{num, number} {num, plural, one {member} other {members}}'
@@ -235,10 +235,12 @@ export default class AddGroupsToTeamModal extends React.Component {
 
  let groupsToShow = this.props.groups;
  if (this.props.excludeGroups) {
- groupsToShow = groupsToShow.filter((g) => !this.props.excludeGroups.includes(g));
+ const hasGroup = (og) => !this.props.excludeGroups.find((g) => g.id === og.id);
+ groupsToShow = groupsToShow.filter(hasGroup);
  }
  if (this.props.includeGroups) {
- groupsToShow = [...groupsToShow, ...this.props.includeGroups.filter((g) => !groupsToShow.includes(g))];
+ const hasGroup = (og) => this.props.includeGroups.find((g) => g.id === og.id);
+ groupsToShow = [...groupsToShow, ...this.props.includeGroups.filter(hasGroup)];
  }
 
  return (

components/admin_console/team_channel_settings/abstract_list.jsx

@@ -73,7 +73,8 @@ export default class AbstractList extends React.PureComponent {
  </div>
  );
  }
- return this.props.data.slice(0, PAGE_SIZE).map(this.props.renderRow);
+ const offset = this.state.page * PAGE_SIZE;
+ return this.props.data.slice(offset, offset + PAGE_SIZE).map(this.props.renderRow);
  }
 
  performSearch = (page) => {
@@ -106,12 +107,12 @@ export default class AbstractList extends React.PureComponent {
  const lastPage = endCount === total;
  const firstPage = this.state.page === 0;
  return (
- <div className='groups-list'>
- {this.props.header}
+ <div className='groups-list groups-list-no-padding'>
+ {total > 0 && this.props.header}
  <div className='groups-list--body'>
  {this.renderRows()}
  </div>
- <div className='groups-list--footer'>
+ {total > 0 && <div className='groups-list--footer'>
  <div className='counter'>
  <FormattedMessage
  id='admin.team_channel_settings.list.paginatorCount'
@@ -137,7 +138,7 @@ export default class AbstractList extends React.PureComponent {
  >
  <NextIcon/>
  </button>
- </div>
+ </div>}
  </div>
  );
  }

components/admin_console/team_channel_settings/channel/details/__snapshots__/channel_details.test.jsx.snap

@@ -42,8 +42,14 @@ exports[`admin_console/team_channel_settings/channel/ChannelDetails should match
  "type": "O",
  }
  }
+ team={
+ Object {
+ "display_name": "test",
+ }
+ }
  />
  <ChannelModes
+ isOriginallyPrivate={false}
  isPublic={true}
  isSynced={false}
  onToggle={[Function]}

components/admin_console/team_channel_settings/channel/details/__snapshots__/channel_groups.test.jsx.snap

@@ -37,7 +37,7 @@ exports[`admin_console/team_channel_settings/channel/ChannelGroups should match
  }
  className=""
  id="channel_groups"
- subtitleDefault="Add and remove channel members based on their group membership."
+ subtitleDefault="Add and remove team members based on their group membership on the next scheduled sync."
  subtitleId="admin.channel_settings.channel_detail.syncedGroupsDescription"
  titleDefault="Synced Groups"
  titleId="admin.channel_settings.channel_detail.syncedGroupsTitle"

components/admin_console/team_channel_settings/channel/details/__snapshots__/channel_modes.test.jsx.snap

@@ -21,6 +21,7 @@ exports[`admin_console/team_channel_settings/channel/ChannelModes should match s
  onToggle={[MockFunction]}
  />
  <AllowAllToggle
+ isOriginallyPrivate={false}
  isPublic={true}
  isSynced={false}
  onToggle={[MockFunction]}

components/admin_console/team_channel_settings/channel/details/__snapshots__/channel_profile.test.jsx.snap

@@ -21,6 +21,13 @@ exports[`admin_console/team_channel_settings/channel/ChannelProfile should match
  />
  <br />
  test
+ <br />
+ <InjectIntl(FormattedMarkdownMessage)
+ defaultMessage="**Team**"
+ id="admin.channel_settings.channel_detail.channelTeam"
+ />
+ <br />
+ test
  </div>
  </div>
 </AdminPanel>

components/admin_console/team_channel_settings/channel/details/channel_details.jsx

@@ -23,12 +23,14 @@ export default class ChannelDetails extends React.Component {
  static propTypes = {
  channelID: PropTypes.string.isRequired,
  channel: PropTypes.object.isRequired,
+ team: PropTypes.object.isRequired,
  groups: PropTypes.arrayOf(PropTypes.object).isRequired,
  totalGroups: PropTypes.number.isRequired,
  allGroups: PropTypes.object.isRequired,
  actions: PropTypes.shape({
  getGroups: PropTypes.func.isRequired,
  linkGroupSyncable: PropTypes.func.isRequired,
+ convertChannelToPrivate: PropTypes.func.isRequired,
  unlinkGroupSyncable: PropTypes.func.isRequired,
  membersMinusGroupMembers: PropTypes.func.isRequired,
  setNavigationBlocked: PropTypes.func.isRequired,
@@ -56,17 +58,21 @@ export default class ChannelDetails extends React.Component {
  };
  }
 
- componentDidUpdate(prevProps) { // TODO: find out how to do this without the lifecycle
- if (prevProps.totalGroups !== this.props.totalGroups) {
+ componentDidUpdate(prevProps) {
+ if (this.props.channel.id !== prevProps.channel.id) {
  // eslint-disable-next-line react/no-did-update-set-state
- this.setState({totalGroups: this.props.totalGroups});
+ this.setState({
+ totalGroups: this.props.totalGroups,
+ isSynced: Boolean(this.props.channel.group_constrained),
+ isPublic: this.props.channel.type === Constants.OPEN_CHANNEL,
+ });
  }
  }
 
- componentDidMount() {
+ async componentDidMount() {
  const {channelID, actions} = this.props;
- actions.getChannel(channelID).
- then(() => actions.getGroups(channelID)).
+ actions.getGroups(channelID).
+ then(() => actions.getChannel(channelID)).
  then(() => this.setState({groups: this.props.groups}));
  }
 
@@ -146,21 +152,23 @@ export default class ChannelDetails extends React.Component {
  serverError = <NeedGroupsError/>;
  saveNeeded = true;
  } else {
- const {error} = await actions.patchChannel(channel.id, {
+ const promises = [];
+ if (!isPublic && channel.type === Constants.OPEN_CHANNEL) {
+ promises.push(actions.convertChannelToPrivate(channel.id));
+ }
+ promises.push(actions.patchChannel(channel.id, {
  ...channel,
  group_constrained: isSynced,
  type: isPublic ? Constants.OPEN_CHANNEL : Constants.PRIVATE_CHANNEL,
- });
- if (error) {
- serverError = <FormError error={error.message}/>;
+ }));
+ const unlink = origGroups.filter((g) => !groups.includes(g)).map((g) => actions.unlinkGroupSyncable(g.id, channelID, Groups.SYNCABLE_TYPE_CHANNEL));
+ const link = groups.filter((g) => !origGroups.includes(g)).map((g) => actions.linkGroupSyncable(g.id, channelID, Groups.SYNCABLE_TYPE_CHANNEL, {auto_add: true}));
+ const result = await Promise.all([...promises, ...unlink, ...link]);
+ const resultWithError = result.find((r) => r.error);
+ if (resultWithError) {
+ serverError = <FormError error={resultWithError.error.message}/>;
  } else {
- const unlink = origGroups.filter((g) => !groups.includes(g)).map((g) => actions.unlinkGroupSyncable(g.id, channelID, Groups.SYNCABLE_TYPE_CHANNEL));
- const link = groups.filter((g) => !origGroups.includes(g)).map((g) => actions.linkGroupSyncable(g.id, channelID, Groups.SYNCABLE_TYPE_CHANNEL));
- const result = await Promise.all([...unlink, ...link]);
- const resultWithError = result.find((r) => r.error);
- if (resultWithError) {
- serverError = <FormError error={resultWithError.error.message}/>;
- }
+ await actions.getGroups(channelID);
  }
  }
 
@@ -170,9 +178,9 @@ export default class ChannelDetails extends React.Component {
 
  render = () => {
  const {totalGroups, saving, saveNeeded, serverError, isSynced, isPublic, groups, showRemoveConfirmation, usersToRemove} = this.state;
- const {channel} = this.props;
- const removedGroups = this.props.groups.filter((g) => !groups.includes(g));
-
+ const {channel, team} = this.props;
+ const missingGroup = (og) => !groups.find((g) => g.id === og.id);
+ const removedGroups = this.props.groups.filter(missingGroup);
  return (
  <div className='wrapper--fixed'>
  <div className='admin-console__header with-back'>
@@ -196,11 +204,15 @@ export default class ChannelDetails extends React.Component {
  onCancel={this.hideRemoveUsersModal}
  onConfirm={this.handleSubmit}
  />
- <ChannelProfile channel={channel}/>
+ <ChannelProfile
+ channel={channel}
+ team={team}
+ />
 
  <ChannelModes
  isPublic={isPublic}
  isSynced={isSynced}
+ isOriginallyPrivate={channel.type === Constants.PRIVATE_CHANNEL}
  onToggle={this.setToggles}
  />
 

components/admin_console/team_channel_settings/channel/details/channel_details.test.jsx

@@ -23,13 +23,20 @@ describe('admin_console/team_channel_settings/channel/ChannelDetails', () => {
  group_constrained: false,
  name: 'DN',
  };
+ const team = {
+ display_name: 'test',
+ };
+
  const wrapper = shallow(
  <ChannelDetails
  groups={groups}
+ team={team}
  totalGroups={groups.length}
  actions={{
  getChannel: jest.fn().mockResolvedValue([]),
+ convertChannelToPrivate: jest.fn(),
  linkGroupSyncable: jest.fn(),
+ conver: jest.fn(),
  patchChannel: jest.fn(),
  setNavigationBlocked: jest.fn(),
  unlinkGroupSyncable: jest.fn(),

components/admin_console/team_channel_settings/channel/details/channel_groups.jsx

@@ -18,7 +18,7 @@ export const ChannelGroups = ({onGroupRemoved, onAddCallback, totalGroups, group
  titleId={synced ? t('admin.channel_settings.channel_detail.syncedGroupsTitle') : t('admin.channel_settings.channel_detail.groupsTitle')}
  titleDefault={synced ? 'Synced Groups' : 'Groups'}
  subtitleId={synced ? t('admin.channel_settings.channel_detail.syncedGroupsDescription') : t('admin.channel_settings.channel_detail.groupsDescription')}
- subtitleDefault={synced ? 'Add and remove channel members based on their group membership.' : 'Group members will be added to the channel.'}
+ subtitleDefault={synced ? 'Add and remove team members based on their group membership on the next scheduled sync.' : 'Group members will be added to the channel based on your sync schedule.'}
  button={
  <ToggleModalButton
  className='btn btn-primary'