From 9b0da552e7b60b1689e81dca24c494d3d6abd50e Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Tue, 1 Feb 2022 11:23:21 -0600
Subject: [PATCH] Fix vertical spacing in `compact` `<ContextMenu>` (#7684)

Fix https://github.com/vector-im/element-web/issues/20801

Regressed in https://github.com/matrix-org/matrix-react-sdk/pull/7339

Relevant styles were first added in https://github.com/matrix-org/matrix-react-sdk/pull/4858
(context behind why the original styles were added)

---

## Cause

Battling CSS specificity between the default and compact styles, https://specificity.keegan.st/

Known good (On `app.element.io` (expected)):
```css
// 0 3 0
.mx_IconizedContextMenu .mx_IconizedContextMenu_optionList .mx_AccessibleButton {
    padding-top: 12px;
    padding-bottom: 12px;
}

// Compact styles override our default rules because they come
// after the other styles (source order) and have the same specificity
// 0 3 0
.mx_IconizedContextMenu.mx_IconizedContextMenu_compact .mx_IconizedContextMenu_optionList > * {
    padding: 8px 16px 8px 11px;
}
```

Bad (On `develop` (broken)):
```css
// Default rules always override because they have higher specificity.
// The `:not()` selector doesn't add any extra specificity but the selectors inside the `:not(...)` do.
// 0 4 0
.mx_IconizedContextMenu .mx_IconizedContextMenu_optionList .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind) {
    padding-top: 12px;
    padding-bottom: 12px;
}

// 0 3 0
.mx_IconizedContextMenu.mx_IconizedContextMenu_compact .mx_IconizedContextMenu_optionList > * {
    padding: 8px 16px 8px 11px;
}
```
---
 .../context_menus/_IconizedContextMenu.scss    | 12 ++++++------
 res/css/views/messages/_JumpToDatePicker.scss  |  4 ++++
 .../context_menus/IconizedContextMenu.tsx      | 18 ++++++++++++++++--
 .../views/messages/DateSeparator.tsx           |  1 -
 4 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/res/css/views/context_menus/_IconizedContextMenu.scss b/res/css/views/context_menus/_IconizedContextMenu.scss
index 9dfda3b013a..01ca8a419d8 100644
--- a/res/css/views/context_menus/_IconizedContextMenu.scss
+++ b/res/css/views/context_menus/_IconizedContextMenu.scss
@@ -50,21 +50,21 @@ limitations under the License.
         }
 
         // round the top corners of the top button for the hover effect to be bounded
-        &:first-child .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind):first-child {
+        &:first-child .mx_IconizedContextMenu_item:first-child {
             border-radius: 8px 8px 0 0; // radius matches .mx_ContextualMenu
         }
 
         // round the bottom corners of the bottom button for the hover effect to be bounded
-        &:last-child .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind):last-child {
+        &:last-child .mx_IconizedContextMenu_item:last-child {
             border-radius: 0 0 8px 8px; // radius matches .mx_ContextualMenu
         }
 
         // round all corners of the only button for the hover effect to be bounded
-        &:first-child:last-child .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind):first-child:last-child {
+        &:first-child:last-child .mx_IconizedContextMenu_item:first-child:last-child {
             border-radius: 8px; // radius matches .mx_ContextualMenu
         }
 
-        .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind) {
+        .mx_IconizedContextMenu_item {
             // pad the inside of the button so that the hover background is padded too
             padding-top: 12px;
             padding-bottom: 12px;
@@ -130,7 +130,7 @@ limitations under the License.
     }
 
     .mx_IconizedContextMenu_optionList_red {
-        .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind) {
+        .mx_IconizedContextMenu_item {
             color: $alert !important;
         }
 
@@ -148,7 +148,7 @@ limitations under the License.
     }
 
     .mx_IconizedContextMenu_active {
-        &.mx_AccessibleButton:not(.mx_AccessibleButton_hasKind), .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind) {
+        &.mx_IconizedContextMenu_item, .mx_IconizedContextMenu_item {
             color: $accent !important;
         }
 
diff --git a/res/css/views/messages/_JumpToDatePicker.scss b/res/css/views/messages/_JumpToDatePicker.scss
index 5722ed1306e..d9b6fb9e91f 100644
--- a/res/css/views/messages/_JumpToDatePicker.scss
+++ b/res/css/views/messages/_JumpToDatePicker.scss
@@ -16,6 +16,10 @@ limitations under the License.
 
 .mx_JumpToDatePicker_form {
     display: flex;
+    // This matches the default padding of IconizedContextMenuOption
+    // (see context_menus/_IconizedContextMenu.scss)
+    padding-top: 12px;
+    padding-bottom: 12px;
 }
 
 .mx_JumpToDatePicker_label {
diff --git a/src/components/views/context_menus/IconizedContextMenu.tsx b/src/components/views/context_menus/IconizedContextMenu.tsx
index cbe9d5fcc9b..2c6bdb3776b 100644
--- a/src/components/views/context_menus/IconizedContextMenu.tsx
+++ b/src/components/views/context_menus/IconizedContextMenu.tsx
@@ -59,6 +59,7 @@ export const IconizedContextMenuRadio: React.FC<IRadioProps> = ({
     return <MenuItemRadio
         {...props}
         className={classNames(className, {
+            mx_IconizedContextMenu_item: true,
             mx_IconizedContextMenu_active: active,
         })}
         active={active}
@@ -93,6 +94,7 @@ export const IconizedContextMenuCheckbox: React.FC<ICheckboxProps> = ({
     return <MenuItemCheckbox
         {...props}
         className={classNames(className, {
+            mx_IconizedContextMenu_item: true,
             mx_IconizedContextMenu_active: active,
         })}
         active={active}
@@ -104,8 +106,20 @@ export const IconizedContextMenuCheckbox: React.FC<ICheckboxProps> = ({
     </MenuItemCheckbox>;
 };
 
-export const IconizedContextMenuOption: React.FC<IOptionProps> = ({ label, iconClassName, children, ...props }) => {
-    return <MenuItem {...props} label={label}>
+export const IconizedContextMenuOption: React.FC<IOptionProps> = ({
+    label,
+    className,
+    iconClassName,
+    children,
+    ...props
+}) => {
+    return <MenuItem
+        {...props}
+        className={classNames(className, {
+            mx_IconizedContextMenu_item: true,
+        })}
+        label={label}
+    >
         { iconClassName && <span className={classNames("mx_IconizedContextMenu_icon", iconClassName)} /> }
         <span className="mx_IconizedContextMenu_label">{ label }</span>
         { children }
diff --git a/src/components/views/messages/DateSeparator.tsx b/src/components/views/messages/DateSeparator.tsx
index 76040c1ad75..d0809c63d85 100644
--- a/src/components/views/messages/DateSeparator.tsx
+++ b/src/components/views/messages/DateSeparator.tsx
@@ -193,7 +193,6 @@ export default class DateSeparator extends React.Component<IProps, IState> {
         if (this.state.contextMenuPosition) {
             contextMenu = <IconizedContextMenu
                 {...contextMenuBelow(this.state.contextMenuPosition)}
-                compact
                 onFinished={this.onContextMenuCloseClick}
             >
                 <IconizedContextMenuOptionList first>