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

Fix vertical spacing in compact <ContextMenu> #7684

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
Copy link
Contributor Author

@MadLittleMods MadLittleMods Feb 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now styling a selector that we actually control from this component (.mx_IconizedContextMenu_item) instead of overriding .mx_AccessibleButton which we use for all <MenuItem>.

This still allows us to nest buttons inside context menus without the need for .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind). For example the nested Go button in the context menu which should use the primary button styles and not the context menu item styles.

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
Copy link
Contributor Author

@MadLittleMods MadLittleMods Feb 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing as the jump to date context menu shouldn't be compact.

Previously was not compact either because of this regression we're fixing in this PR which made compact context menu's use the default styles.

onFinished={this.onContextMenuCloseClick}
>
<IconizedContextMenuOptionList first>
Expand Down