[Maps] Improving design and a11y for attribution layer settings (#38)

* Design and a11y improvements

* Buttons aria labels

* Addressing PR review

x-pack/plugins/maps/public/connected_components/layer_panel/_index.scss

@@ -1,5 +1,5 @@
 @import 'layer_panel';
-@import 'layer_settings/attribution_popover';
+@import 'layer_settings/index';
 @import 'filter_editor/filter_editor';
 @import 'join_editor/resources/join';
 @import 'style_settings/style_settings';

x-pack/plugins/maps/public/connected_components/layer_panel/layer_settings/_attribution_form_row.scss

@@ -0,0 +1,27 @@
+.mapAttributionFormRow {
+  display: flex;
+  margin: $euiSizeS 0;
+}
+
+.mapAttributionFormRow__legend {
+  @include euiFormLabel;
+  display: inline-flex;
+  width: calc(33% - #{$euiSizeS});
+  height: $euiSizeXL;
+  align-items: center;
+  margin-right: $euiSizeS;
+}
+
+.mapAttributionFormRow__field {
+  width: 67%;
+}
+
+.mapAttributionFormRow__addButton {
+  margin-top: $euiSizeXS;
+}
+
+.mapAttributionFormRow__buttons {
+  display: flex;
+  justify-content: flex-end;
+  margin-top: $euiSizeXS;
+}

x-pack/plugins/maps/public/connected_components/layer_panel/layer_settings/_attribution_popover.scss

@@ -1,3 +1,3 @@
 .mapAttributionPopover {
   width: $euiSizeXXL * 12;
-}
+}

x-pack/plugins/maps/public/connected_components/layer_panel/layer_settings/_index.scss

@@ -0,0 +1,2 @@
+@import 'attribution_form_row';
+@import 'attribution_popover';

x-pack/plugins/maps/public/connected_components/layer_panel/layer_settings/attribution_form_row.tsx

@@ -6,7 +6,7 @@
  */
 
 import React from 'react';
-import { EuiButtonEmpty, EuiFlexGrid, EuiFlexItem, EuiFormRow, EuiLink } from '@elastic/eui';
+import { EuiButtonEmpty, EuiFlexItem, EuiTitle, EuiLink, EuiPanel } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n/react';
 import { Attribution } from '../../../../common/descriptor_types';
@@ -21,57 +21,77 @@ interface Props {
 export function AttributionFormRow(props: Props) {
   function renderAttribution() {
     const layerDescriptor = props.layer.getDescriptor();
-    return layerDescriptor.attribution === undefined ? (
-      <AttributionPopover
-        onChange={props.onChange}
-        popoverButtonIcon="plusInCircleFilled"
-        popoverButtonLabel={i18n.translate('xpack.maps.attribution.addBtnLabel', {
-          defaultMessage: 'Add',
-        })}
-        label={''}
-        url={''}
-      />
-    ) : (
-      <div>
-        <EuiLink color="text" href={layerDescriptor.attribution.url} target="_blank">
-          {layerDescriptor.attribution.label}
-        </EuiLink>
-        <EuiFlexGrid>
-          <EuiFlexItem grow={false}>
-            <AttributionPopover
-              onChange={props.onChange}
-              popoverButtonIcon="pencil"
-              popoverButtonLabel={i18n.translate('xpack.maps.attribution.editBtnLabel', {
-                defaultMessage: 'Edit',
-              })}
-              label={layerDescriptor.attribution.label}
-              url={layerDescriptor.attribution.url}
-            />
-          </EuiFlexItem>
-          <EuiFlexItem grow={false}>
-            <EuiButtonEmpty
-              onClick={() => {
-                props.onChange();
-              }}
-              size="xs"
-              iconType="trash"
-            >
-              <FormattedMessage id="xpack.maps.attribution.clearBtnLabel" defaultMessage="Clear" />
-            </EuiButtonEmpty>
-          </EuiFlexItem>
-        </EuiFlexGrid>
-      </div>
+
+    return (
+      <fieldset aria-labelledby="mapsLayerSettingsAttributionLegend">
+        <div className="mapAttributionFormRow">
+          <legend id="mapsLayerSettingsAttributionLegend" className="mapAttributionFormRow__legend">
+            {i18n.translate('xpack.maps.layerSettings.attributionLegend', {
+              defaultMessage: 'Attribution',
+            })}
+          </legend>
+
+          {layerDescriptor.attribution === undefined ? (
+            <div className="mapAttributionFormRow__field">
+              <AttributionPopover
+                onChange={props.onChange}
+                popoverButtonIcon="plusInCircleFilled"
+                popoverButtonLabel={i18n.translate('xpack.maps.attribution.addBtnLabel', {
+                  defaultMessage: 'Add attribution',
+                })}
+                popoverButtonAriaLabel={i18n.translate('xpack.maps.attribution.addBtnAriaLabel', {
+                  defaultMessage: 'Add attribution',
+                })}
+                popoverButtonClassName="mapAttributionFormRow__addButton"
+                label={''}
+                url={''}
+              />
+            </div>
+          ) : (
+            <div className="mapAttributionFormRow__field">
+              <EuiPanel color="subdued" paddingSize="s">
+                <EuiLink color="text" href={layerDescriptor.attribution.url} target="_blank">
+                  {layerDescriptor.attribution.label}
+                </EuiLink>
+              </EuiPanel>
+
+              <div className="mapAttributionFormRow__buttons">
+                <AttributionPopover
+                  onChange={props.onChange}
+                  popoverButtonIcon="pencil"
+                  popoverButtonAriaLabel={i18n.translate('xpack.maps.attribution.addBtnAriaLabel', {
+                    defaultMessage: 'Edit attribution',
+                  })}
+                  popoverButtonLabel={i18n.translate('xpack.maps.attribution.editBtnLabel', {
+                    defaultMessage: 'Edit',
+                  })}
+                  label={layerDescriptor.attribution.label}
+                  url={layerDescriptor.attribution.url}
+                />
+
+                <EuiButtonEmpty
+                  onClick={() => {
+                    props.onChange();
+                  }}
+                  size="xs"
+                  iconType="trash"
+                  color="danger"
+                  aria-label={i18n.translate('xpack.maps.attribution.clearBtnAriaLabel', {
+                    defaultMessage: 'Clear attribution',
+                  })}
+                >
+                  <FormattedMessage
+                    id="xpack.maps.attribution.clearBtnLabel"
+                    defaultMessage="Clear"
+                  />
+                </EuiButtonEmpty>
+              </div>
+            </div>
+          )}
+        </div>
+      </fieldset>
     );
   }
 
-  return props.layer.getSource().getAttributionProvider() ? null : (
-    <EuiFormRow
-      label={i18n.translate('xpack.maps.layerSettings.attributionLabel', {
-        defaultMessage: 'Attribution',
-      })}
-      display="columnCompressed"
-    >
-      {renderAttribution()}
-    </EuiFormRow>
-  );
+  return props.layer.getSource().getAttributionProvider() ? null : renderAttribution();
 }

x-pack/plugins/maps/public/connected_components/layer_panel/layer_settings/attribution_popover.tsx

@@ -24,7 +24,9 @@ import { Attribution } from '../../../../common/descriptor_types';
 interface Props {
   onChange: (attribution: Attribution) => void;
   popoverButtonLabel: string;
+  popoverButtonAriaLabel: string;
   popoverButtonIcon: string;
+  popoverButtonClassName?: string;
   label: string;
   url: string;
 }
@@ -73,9 +75,12 @@ export class AttributionPopover extends Component<Props, State> {
   _renderPopoverButton() {
     return (
       <EuiButtonEmpty
+        className={this.props.popoverButtonClassName}
+        aria-label={this.props.popoverButtonAriaLabel}
         onClick={this._togglePopover}
         size="xs"
         iconType={this.props.popoverButtonIcon}
+        flush="left"
       >
         {this.props.popoverButtonLabel}
       </EuiButtonEmpty>
@@ -99,15 +104,28 @@ export class AttributionPopover extends Component<Props, State> {
           })}
           fullWidth
         >
-          <EuiFieldText fullWidth value={this.state.label} onChange={this._onLabelChange} />
+          <EuiFieldText
+            compressed
+            fullWidth
+            value={this.state.label}
+            onChange={this._onLabelChange}
+          />
         </EuiFormRow>
         <EuiFormRow
           label={i18n.translate('xpack.maps.attribution.urlLabel', {
             defaultMessage: 'Link',
           })}
           fullWidth
         >
-          <EuiFieldText fullWidth value={this.state.url} onChange={this._onUrlChange} />
+          <EuiFieldText
+            compressed
+            fullWidth
+            value={this.state.url}
+            onChange={this._onUrlChange}
+            placeholder={i18n.translate('xpack.maps.attribution.urlPlaceholder', {
+              defaultMessage: 'http://myurl.com',
+            })}
+          />
         </EuiFormRow>
         <EuiSpacer size="xs" />
         <EuiPopoverFooter>
@@ -130,6 +148,7 @@ export class AttributionPopover extends Component<Props, State> {
     return (
       <EuiPopover
         id="attributionPopover"
+        panelPaddingSize="s"
         anchorPosition="leftCenter"
         button={this._renderPopoverButton()}
         isOpen={this.state.isPopoverOpen}