Revert #2231 (#2377)

* Revert 2231
Shopify · Oct 31, 2019 · 47d0240 · 47d0240
1 parent 500347c
commit 47d0240
Show file tree

Hide file tree

Showing 9 changed files with 19 additions and 64 deletions.
diff --git a/UNRELEASED.md b/UNRELEASED.md
@@ -12,6 +12,10 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 
 ### Bug fixes
 
+- Fixed an accessibility issue with `TextField` `multiline` where `aria-multiline` would be set to an invalid type `number` ([#2351](https://github.com/Shopify/polaris-react/pull/2351))
+- Revert [#2231](https://github.com/Shopify/polaris-react/pull/2351) as it breaks middle aligned popovers ([#2237](https://github.com/Shopify/polaris-react/pull/2237))
+- Fixed alignement of disclosure icons on `ResourceItem` ([#2370](https://github.com/Shopify/polaris-react/pull/2370))
+
 ### Documentation
 
 ### Development workflow

diff --git a/src/components/Popover/Popover.scss b/src/components/Popover/Popover.scss
@@ -11,7 +11,6 @@ $content-max-width: rem(400px);
   box-shadow: shadow(deep);
   border-radius: var(--p-border-radius-wide, border-radius());
   will-change: left, top;
-  width: 100%;
 }
 
 .PopoverOverlay {
@@ -53,7 +52,7 @@ $content-max-width: rem(400px);
 }
 
 .positionedAbove {
-  margin: spacing() spacing(tight) $visible-portion-of-arrow;
+  margin: spacing() 0 $visible-portion-of-arrow spacing(tight);
 
   &.fullWidth {
     margin: 0 0 $visible-portion-of-arrow;

diff --git a/src/components/PositionedOverlay/PositionedOverlay.tsx b/src/components/PositionedOverlay/PositionedOverlay.tsx
@@ -44,7 +44,6 @@ export interface PositionedOverlayProps {
 interface State {
   measuring: boolean;
   activatorRect: Rect;
-  right?: number;
   left: number;
   top: number;
   height: number;
@@ -119,23 +118,12 @@ export class PositionedOverlay extends React.PureComponent<
   }
 
   render() {
-    const {top, zIndex, width} = this.state;
+    const {left, top, zIndex, width} = this.state;
     const {render, fixed, classNames: propClassNames} = this.props;
 
-    const right =
-      this.state.right == null || isNaN(this.state.right)
-        ? undefined
-        : this.state.right;
-
-    const left =
-      right != null || this.state.left == null || isNaN(this.state.left)
-        ? undefined
-        : this.state.left;
-
     const style = {
-      right,
-      left,
       top: top == null || isNaN(top) ? undefined : top,
+      left: left == null || isNaN(left) ? undefined : left,
       width: width == null || isNaN(width) ? undefined : width,
       zIndex: zIndex == null || isNaN(zIndex) ? undefined : zIndex,
     };
@@ -246,9 +234,7 @@ export class PositionedOverlay extends React.PureComponent<
           {
             measuring: false,
             activatorRect: getRectForNode(activator),
-            right:
-              preferredAlignment === 'right' ? horizontalPosition : undefined,
-            left: preferredAlignment === 'right' ? 0 : horizontalPosition,
+            left: horizontalPosition,
             top: lockPosition ? top : verticalPosition.top,
             lockPosition: Boolean(fixed),
             height: verticalPosition.height || 0,

diff --git a/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx b/src/components/PositionedOverlay/tests/PositionedOverlay.test.tsx
@@ -64,7 +64,7 @@ describe('<PositionedOverlay />', () => {
   });
 
   describe('preferredAlignment', () => {
-    it('aligns left if preferredAlignment left is given', () => {
+    it('aligns left if preferredAlignment is given', () => {
       const positionedOverlay = mountWithAppProvider(
         <PositionedOverlay {...mockProps} preferredAlignment="left" />,
       );
@@ -73,16 +73,6 @@ describe('<PositionedOverlay />', () => {
         (positionedOverlay.find('div').prop('style') as any).left,
       ).toBeUndefined();
     });
-
-    it('aligns right if preferredAlignment right is given', () => {
-      const positionedOverlay = mountWithAppProvider(
-        <PositionedOverlay {...mockProps} preferredAlignment="right" />,
-      );
-
-      expect(
-        (positionedOverlay.find('div').prop('style') as any).right,
-      ).toBeUndefined();
-    });
   });
 
   describe('fullWidth', () => {

diff --git a/src/components/PositionedOverlay/utilities/math.ts b/src/components/PositionedOverlay/utilities/math.ts
@@ -91,7 +91,6 @@ export function calculateHorizontalPosition(
   preferredAlignment: PreferredAlignment,
 ) {
   const maximum = containerRect.width - overlayRect.width;
-  const borderWidth = 2;
 
   if (preferredAlignment === 'left') {
     return Math.min(
@@ -100,26 +99,18 @@ export function calculateHorizontalPosition(
     );
   } else if (preferredAlignment === 'right') {
     const activatorRight = activatorRect.left + activatorRect.width;
-
-    return (
-      maximum -
+    return Math.min(
+      maximum,
       Math.max(
         0,
-        activatorRight -
-          (overlayRect.width - overlayMargins.horizontal) -
-          borderWidth,
-      )
+        activatorRight - overlayRect.width + overlayMargins.horizontal,
+      ),
     );
   }
 
   return Math.min(
     maximum,
-    Math.max(
-      0,
-      activatorRect.center.x -
-        overlayRect.width / 2 -
-        overlayMargins.horizontal,
-    ),
+    Math.max(0, activatorRect.center.x - overlayRect.width / 2),
   );
 }
 

diff --git a/src/components/TopBar/TopBar.scss b/src/components/TopBar/TopBar.scss
@@ -11,7 +11,6 @@ $context-control-expand-after: 1400px;
   display: flex;
   height: top-bar-height();
   background-color: var(--p-surface-background, var(--top-bar-background));
-
   &::after {
     content: '';
     position: absolute;

diff --git a/src/components/TopBar/components/Menu/Menu.scss b/src/components/TopBar/components/Menu/Menu.scss
@@ -18,11 +18,6 @@ $activator-variables: (
   height: top-bar-height();
   display: flex;
   align-items: center;
-  margin-right: spacing(tight);
-
-  @include breakpoint-before(layout-width(page-with-nav), false) {
-    margin: 0 spacing(extra-tight);
-  }
 }
 
 .Activator {
@@ -37,6 +32,7 @@ $activator-variables: (
   border: 0;
   cursor: pointer;
   transition: menu(transition);
+  margin-right: spacing(tight);
   border-radius: border-radius();
 
   &:focus {
@@ -56,6 +52,8 @@ $activator-variables: (
   }
 
   @include breakpoint-before(layout-width(page-with-nav), false) {
+    margin: 0 spacing(extra-tight);
+
     &:focus,
     &:hover,
     &:active,

diff --git a/src/components/TopBar/components/Menu/Menu.tsx b/src/components/TopBar/components/Menu/Menu.tsx
@@ -1,7 +1,7 @@
 import React from 'react';
 
 import {ActionList, ActionListProps} from '../../../ActionList';
-import {Popover, PopoverProps} from '../../../Popover';
+import {Popover} from '../../../Popover';
 
 import {Message, MessageProps} from './components';
 import styles from './Menu.scss';
@@ -15,24 +15,14 @@ export interface MenuProps {
   message?: MessageProps;
   /** A boolean property indicating whether the menu is currently open */
   open: boolean;
-  /** The preferred alignment of the popover relative to its activator */
-  preferredAlignment?: PopoverProps['preferredAlignment'];
   /** A callback function to handle opening the menu popover */
   onOpen(): void;
   /** A callback function to handle closing the menu popover */
   onClose(): void;
 }
 
 export function Menu(props: MenuProps) {
-  const {
-    actions,
-    onOpen,
-    onClose,
-    open,
-    activatorContent,
-    message,
-    preferredAlignment,
-  } = props;
+  const {actions, onOpen, onClose, open, activatorContent, message} = props;
 
   const badgeProps = message &&
     message.badge && {
@@ -56,7 +46,6 @@ export function Menu(props: MenuProps) {
 
   return (
     <Popover
-      preferredAlignment={preferredAlignment}
       activator={
         <div className={styles.ActivatorWrapper}>
           <button type="button" className={styles.Activator} onClick={onOpen}>

diff --git a/src/components/TopBar/components/UserMenu/UserMenu.tsx b/src/components/TopBar/components/UserMenu/UserMenu.tsx
@@ -55,7 +55,6 @@ export function UserMenu({
 
   return (
     <Menu
-      preferredAlignment="right"
       activatorContent={activatorContentMarkup}
       open={open}
       onOpen={onToggle}