Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
Fix vertical spacing in compact <ContextMenu> (#7684)
Browse files Browse the repository at this point in the history
Fix element-hq/element-web#20801

Regressed in #7339

Relevant styles were first added in #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;
}
```
  • Loading branch information
MadLittleMods committed Feb 1, 2022
1 parent 82f3888 commit 9b0da55
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 9 deletions.
12 changes: 6 additions & 6 deletions res/css/views/context_menus/_IconizedContextMenu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -130,7 +130,7 @@ limitations under the License.
}

.mx_IconizedContextMenu_optionList_red {
.mx_AccessibleButton:not(.mx_AccessibleButton_hasKind) {
.mx_IconizedContextMenu_item {
color: $alert !important;
}

Expand All @@ -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;
}

Expand Down
4 changes: 4 additions & 0 deletions res/css/views/messages/_JumpToDatePicker.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 16 additions & 2 deletions src/components/views/context_menus/IconizedContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand All @@ -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 }
Expand Down
1 change: 0 additions & 1 deletion src/components/views/messages/DateSeparator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit 9b0da55

Please sign in to comment.