Standardize chip height (#213)

* make all chips the same size

* create DetailList, Detail components; clean up Chips, ChipGroup

* delete BasicChip in favor of <Chip isReadOnly>

* create our own ChipGroup to handle overflow

src/pages/Organizations/components/OrganizationAccessItem.jsx

@@ -5,55 +5,21 @@ import { t } from '@lingui/macro';
 import {
   DataListItem,
   DataListItemRow,
-  DataListItemCells,
+  DataListItemCells as PFDataListItemCells,
   DataListCell,
   Text,
   TextContent,
   TextVariants,
-  Chip,
 } from '@patternfly/react-core';
 import { Link } from 'react-router-dom';
+import styled from 'styled-components';
 import { AccessRecord } from '../../../types';
-import BasicChip from '../../../components/BasicChip/BasicChip';
+import { DetailList, Detail } from '../../../components/DetailList';
+import { ChipGroup, Chip } from '../../../components/Chip';
 
-const userRolesWrapperStyle = {
-  display: 'flex',
-  flexWrap: 'wrap',
-};
-
-const detailWrapperStyle = {
-  display: 'grid',
-  gridTemplateColumns: 'minmax(70px, max-content) minmax(60px, max-content)',
-};
-
-const detailLabelStyle = {
-  fontWeight: '700',
-  lineHeight: '24px',
-  marginRight: '20px',
-};
-
-const detailValueStyle = {
-  lineHeight: '28px',
-  overflow: 'visible',
-};
-
-/* TODO: does PF offer any sort of <dl> treatment for this? */
-const Detail = ({ label, value, url, customStyles }) => {
-  let detail = null;
-  if (value) {
-    detail = (
-      <TextContent style={{ ...detailWrapperStyle, ...customStyles }}>
-        {url ? (
-          <Link to={{ pathname: url }}>
-            <Text component={TextVariants.h6} style={detailLabelStyle}>{label}</Text>
-          </Link>) : (<Text component={TextVariants.h6} style={detailLabelStyle}>{label}</Text>
-        )}
-        <Text component={TextVariants.p} style={detailValueStyle}>{value}</Text>
-      </TextContent>
-    );
-  }
-  return detail;
-};
+const DataListItemCells = styled(PFDataListItemCells)`
+  align-items: start;
+`;
 
 class OrganizationAccessItem extends React.Component {
   static propTypes = {
@@ -61,6 +27,11 @@ class OrganizationAccessItem extends React.Component {
     onRoleDelete: func.isRequired,
   };
 
+  constructor (props) {
+    super(props);
+    this.renderChip = this.renderChip.bind(this);
+  }
+
   getRoleLists () {
     const { accessRecord } = this.props;
     const teamRoles = [];
@@ -80,8 +51,21 @@ class OrganizationAccessItem extends React.Component {
     return [teamRoles, userRoles];
   }
 
+  renderChip (role) {
+    const { accessRecord, onRoleDelete } = this.props;
+    return (
+      <Chip
+        key={role.id}
+        isReadOnly={!role.user_capabilities.unattach}
+        onClick={() => { onRoleDelete(role, accessRecord); }}
+      >
+        {role.name}
+      </Chip>
+    );
+  }
+
   render () {
-    const { accessRecord, onRoleDelete, i18n } = this.props;
+    const { accessRecord, i18n } = this.props;
     const [teamRoles, userRoles] = this.getRoleLists();
 
     return (
@@ -90,76 +74,60 @@ class OrganizationAccessItem extends React.Component {
           <DataListItemCells dataListCells={[
             <DataListCell key="name">
               {accessRecord.username && (
-                <TextContent style={detailWrapperStyle}>
+                <TextContent>
                   {accessRecord.url ? (
-                    <Link to={{ pathname: accessRecord.url }}>
-                      <Text component={TextVariants.h6} style={detailLabelStyle}>
+                    <Text component={TextVariants.h6}>
+                      <Link
+                        to={{ pathname: accessRecord.url }}
+                        css="font-weight: bold"
+                      >
                         {accessRecord.username}
-                      </Text>
-                    </Link>
+                      </Link>
+                    </Text>
                   ) : (
-                    <Text component={TextVariants.h6} style={detailLabelStyle}>
+                    <Text
+                      component={TextVariants.h6}
+                      css="font-weight: bold"
+                    >
                       {accessRecord.username}
                     </Text>
                   )}
                 </TextContent>
               )}
               {accessRecord.first_name || accessRecord.last_name ? (
-                <Detail
-                  label={i18n._(t`Name`)}
-                  value={`${accessRecord.first_name} ${accessRecord.last_name}`}
-                  url={null}
-                  customStyles={null}
-                />
+                <DetailList stacked>
+                  <Detail
+                    label={i18n._(t`Name`)}
+                    value={`${accessRecord.first_name} ${accessRecord.last_name}`}
+                  />
+                </DetailList>
               ) : (
                 null
               )}
             </DataListCell>,
             <DataListCell key="roles">
-              {userRoles.length > 0 && (
-                <ul style={userRolesWrapperStyle}>
-                  <Text component={TextVariants.h6} style={detailLabelStyle}>
-                    {i18n._(t`User Roles`)}
-                  </Text>
-                  {userRoles.map(role => (
-                    role.user_capabilities.unattach ? (
-                      <Chip
-                        key={role.id}
-                        className="awx-c-chip"
-                        onClick={() => { onRoleDelete(role, accessRecord); }}
-                      >
-                        {role.name}
-                      </Chip>
-                    ) : (
-                      <BasicChip key={role.id}>
-                        {role.name}
-                      </BasicChip>
-                    )
-                  ))}
-                </ul>
-              )}
-              {teamRoles.length > 0 && (
-                <ul style={userRolesWrapperStyle}>
-                  <Text component={TextVariants.h6} style={detailLabelStyle}>
-                    {i18n._(t`Team Roles`)}
-                  </Text>
-                  {teamRoles.map(role => (
-                    role.user_capabilities.unattach ? (
-                      <Chip
-                        key={role.id}
-                        className="awx-c-chip"
-                        onClick={() => { onRoleDelete(role, accessRecord); }}
-                      >
-                        {role.name}
-                      </Chip>
-                    ) : (
-                      <BasicChip key={role.id}>
-                        {role.name}
-                      </BasicChip>
-                    )
-                  ))}
-                </ul>
-              )}
+              <DetailList stacked>
+                {userRoles.length > 0 && (
+                  <Detail
+                    label={i18n._(t`User Roles`)}
+                    value={(
+                      <ChipGroup>
+                        {userRoles.map(this.renderChip)}
+                      </ChipGroup>
+                    )}
+                  />
+                )}
+                {teamRoles.length > 0 && (
+                  <Detail
+                    label={i18n._(t`Team Roles`)}
+                    value={(
+                      <ChipGroup>
+                        {teamRoles.map(this.renderChip)}
+                      </ChipGroup>
+                    )}
+                  />
+                )}
+              </DetailList>
             </DataListCell>
           ]}
           />

src/pages/Organizations/screens/Organization/OrganizationDetail.jsx

@@ -2,75 +2,24 @@ import React, { Component } from 'react';
 import { Link, withRouter } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
-
-import {
-  CardBody as PFCardBody,
-  Button,
-  TextList,
-  TextListItem,
-  TextListVariants,
-  TextListItemVariants,
-} from '@patternfly/react-core';
+import { CardBody as PFCardBody, Button } from '@patternfly/react-core';
 import styled from 'styled-components';
+import { DetailList, Detail } from '../../../../components/DetailList';
 import { withNetwork } from '../../../../contexts/Network';
-import BasicChip from '../../../../components/BasicChip/BasicChip';
+import { ChipGroup, Chip } from '../../../../components/Chip';
 
 const CardBody = styled(PFCardBody)`
   padding-top: 20px;
 `;
 
-const DetailList = styled(TextList)`
-  display: grid;
-  grid-template-columns: repeat(auto-fit, minmax(270px, 1fr));
-  grid-gap: 20px;
-
-  & > div {
-    display: grid;
-    grid-template-columns: 10em 1fr;
-    grid-gap: 20px;
-  }
-`;
-
-const DetailName = styled(TextListItem)`
-  && {
-    grid-column: 1;
-    font-weight: var(--pf-global--FontWeight--bold);
-    text-align: right;
-  }
-`;
-
-const DetailValue = styled(TextListItem)`
-  && {
-    grid-column: 2;
-    word-break: break-all;
-  }
-`;
-
-const InstanceGroupsDetail = styled.div`
-  grid-column: 1 / -1;
-`;
-
-const Detail = ({ label, value }) => {
-  if (!value) return null;
-  return (
-    <div>
-      <DetailName component={TextListItemVariants.dt}>{label}</DetailName>
-      <DetailValue component={TextListItemVariants.dd}>{value}</DetailValue>
-    </div>
-  );
-};
-
 class OrganizationDetail extends Component {
   constructor (props) {
     super(props);
 
     this.state = {
       instanceGroups: [],
-      isToggleOpen: false,
       error: false
     };
-
-    this.handleChipToggle = this.handleChipToggle.bind(this);
     this.loadInstanceGroups = this.loadInstanceGroups.bind(this);
   }
 
@@ -78,12 +27,6 @@ class OrganizationDetail extends Component {
     this.loadInstanceGroups();
   }
 
-  handleChipToggle = () => {
-    this.setState((prevState) => ({
-      isToggleOpen: !prevState.isToggleOpen
-    }));
-  }
-
   async loadInstanceGroups () {
     const {
       api,
@@ -106,7 +49,6 @@ class OrganizationDetail extends Component {
     const {
       error,
       instanceGroups,
-      isToggleOpen,
     } = this.state;
 
     const {
@@ -121,30 +63,10 @@ class OrganizationDetail extends Component {
       match,
       i18n
     } = this.props;
-    const showOverflowChipAfter = 5;
-
-    const instanceGroupChips = instanceGroups.slice(0, isToggleOpen
-      ? instanceGroups.length : showOverflowChipAfter)
-      .map(instanceGroup => (
-        <BasicChip
-          key={instanceGroup.id}
-        >
-          {instanceGroup.name}
-        </BasicChip>
-      ));
-
-    const overflowChip = (instanceGroups.length > showOverflowChipAfter) ? (
-      <BasicChip
-        isOverflowChip
-        onToggle={this.handleChipToggle}
-      >
-        {isToggleOpen ? 'Show less' : `${(instanceGroups.length - showOverflowChipAfter).toString()} more`}
-      </BasicChip>
-    ) : null;
 
     return (
       <CardBody>
-        <DetailList component={TextListVariants.dl}>
+        <DetailList>
           <Detail
             label={i18n._(t`Name`)}
             value={name}
@@ -166,15 +88,17 @@ class OrganizationDetail extends Component {
             value={modified}
           />
           {(instanceGroups && instanceGroups.length > 0) && (
-            <InstanceGroupsDetail>
-              <DetailName component={TextListItemVariants.dt}>
-                {i18n._(t`Instance Groups`)}
-              </DetailName>
-              <DetailValue component={TextListItemVariants.dd}>
-                {instanceGroupChips}
-                {overflowChip}
-              </DetailValue>
-            </InstanceGroupsDetail>
+            <Detail
+              fullWidth
+              label={i18n._(t`Instance Groups`)}
+              value={(
+                <ChipGroup showOverflowAfter={5}>
+                  {instanceGroups.map(ig => (
+                    <Chip key={ig.id} isReadOnly>{ig.name}</Chip>
+                  ))}
+                </ChipGroup>
+              )}
+            />
           )}
         </DetailList>
         {summary_fields.user_capabilities.edit && (