Implement improved spacing for the thread list and timeline (#8337)

Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
matrix-org · Apr 29, 2022 · 12e8534 · 12e8534
1 parent 92f440d
commit 12e8534
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 69 deletions.
diff --git a/res/css/_common.scss b/res/css/_common.scss
@@ -692,6 +692,29 @@ legend {
     }
 }
 
+@define-mixin ThreadsAmount {
+    $threadInfoLineHeight: calc(2 * $font-12px);
+
+    color: $secondary-content;
+    font-weight: $font-semi-bold;
+    line-height: $threadInfoLineHeight;
+    white-space: nowrap;
+    position: relative;
+    padding: 0 $spacing-12 0 $spacing-8;
+}
+
+@define-mixin ThreadInfoIcon {
+    content: "";
+    display: inline-block;
+    mask-image: url('$(res)/img/element-icons/thread-summary.svg');
+    mask-position: center;
+    height: 18px;
+    min-width: 18px;
+    background-color: $secondary-content !important;
+    mask-repeat: no-repeat;
+    mask-size: contain;
+}
+
 @define-mixin ListResetDefault {
     list-style: none;
     padding: 0;

diff --git a/res/css/views/right_panel/_ThreadPanel.scss b/res/css/views/right_panel/_ThreadPanel.scss
@@ -20,10 +20,6 @@ limitations under the License.
     height: 100px;
     overflow: visible;
 
-    &:not(.mx_ThreadView).mx_BaseCard {
-        padding-right: 2px;
-    }
-
     .mx_BaseCard_header {
         margin-bottom: 12px;
 
@@ -111,15 +107,37 @@ limitations under the License.
     .mx_AutoHideScrollbar {
         background-color: $background;
         border-radius: 8px;
-        width: calc(100% - 24px);
-        padding-right: 18px;
+        padding-inline-end: 0;
+        overflow-y: scroll; // set gap between the thread tile and the right border
+    }
+
+    // Override _GroupLayout.scss for the thread panel
+    .mx_GroupLayout {
+        .mx_EventTile {
+            .mx_MessageActionBar {
+                right: 0;
+                top: -36px; // 2px above EventTile
+                z-index: 10; // See _EventTile.scss
+            }
+
+            &[data-shape=ThreadsList] {
+                > .mx_DisambiguatedProfile {
+                    margin-inline-start: 0;
+                }
+
+                .mx_MessageTimestamp {
+                    position: initial;
+                    width: auto;
+                }
+
+                .mx_EventTile_line {
+                    padding-bottom: 0; // Override mx_EventTile_line on _GroupLayout.scss
+                }
+            }
+        }
     }
 
     &.mx_ThreadView .mx_ThreadView_timelinePanelWrapper {
-        /* the scrollbar is 8px wide, and we want a 12px gap with the side of the
-        panel. Hence the magic number, 8+4=12 */
-        width: calc(100% + 6px);
-        padding-right: 4px;
         position: relative;
         min-height: 0; // don't displace the composer
         flex-grow: 1;
@@ -129,9 +147,15 @@ limitations under the License.
         }
     }
 
+    .mx_RoomView_messagePanel { // To avoid the rule from being applied to .mx_ThreadPanel_empty
+        .mx_RoomView_messageListWrapper {
+            width: calc(100% + 6px); // 8px - 2px
+        }
+    }
+
     .mx_RoomView_MessageList {
-        padding-left: 12px;
-        padding-right: 0;
+        padding-inline-start: $spacing-8;
+        padding-inline-end: $spacing-8;
         content-visibility: visible;
     }
 
@@ -256,14 +280,14 @@ limitations under the License.
 }
 
 .mx_ThreadPanel_replies {
-    margin-top: 8px;
+    margin-top: $spacing-8;
     display: flex;
     align-items: center;
     position: relative;
 
-    .mx_ThreadSummary_threads-amount {
-        color: $secondary-content;
-        font-size: $font-12px;
+    .mx_ThreadPanel_ThreadsAmount {
+        @mixin ThreadsAmount;
+        font-size: $font-12px; // Same font size as the counter on the main panel
     }
 }
 
@@ -286,7 +310,6 @@ limitations under the License.
     top: 0;
     bottom: 0;
     left: 0;
-    right: 6px;
     padding: 20px;
 
     h2 {

diff --git a/res/css/views/right_panel/_TimelineCard.scss b/res/css/views/right_panel/_TimelineCard.scss
@@ -42,11 +42,6 @@ limitations under the License.
         border-radius: 8px;
     }
 
-    .mx_AutoHideScrollbar {
-        padding-right: 10px;
-        width: calc(100% - 10px);
-    }
-
     .mx_NewRoomIntro {
         margin-left: 36px;
     }

diff --git a/res/css/views/rooms/_EventTile.scss b/res/css/views/rooms/_EventTile.scss
@@ -16,6 +16,7 @@ limitations under the License.
 */
 
 $left-gutter: 64px;
+$threadInfoLineHeight: calc(2 * $font-12px); // See: _commons.scss
 
 .mx_EventTile {
     flex-shrink: 0;
@@ -683,15 +684,8 @@ $left-gutter: 64px;
 .mx_ThreadPanel_replies::before,
 .mx_ThreadSummaryIcon::before,
 .mx_ThreadSummary::before {
-    content: "";
-    display: inline-block;
-    mask-image: url('$(res)/img/element-icons/thread-summary.svg');
-    mask-position: center;
-    height: 18px;
-    min-width: 18px;
+    @mixin ThreadInfoIcon;
     background-color: $secondary-content !important;
-    mask-repeat: no-repeat;
-    mask-size: contain;
 }
 
 .mx_ThreadSummaryIcon {
@@ -715,28 +709,33 @@ $left-gutter: 64px;
 }
 
 .mx_EventTile[data-shape=ThreadsList] {
-    --topOffset: 20px;
-    --leftOffset: 46px;
+    --topOffset: $spacing-12;
+    --leftOffset: 48px;
     $borderRadius: 8px;
+    $padding: $spacing-8;
+    $hrHeight: 1px;
 
-    margin: var(--topOffset) 16px var(--topOffset) 0;
+    margin: calc(var(--topOffset) + $hrHeight) 0 var(--topOffset); // include the height of horizontal line
+    padding: $padding $spacing-24 $padding $padding;
     border-radius: $borderRadius;
 
     display: flex;
     flex-flow: wrap;
     align-items: center;
 
-    &:hover {
+    &:hover,
+    // To cancel "&.mx_EventTile:hover .mx_EventTile_line"
+    &:not([data-layout=bubble]):hover .mx_EventTile_line {
         background-color: $system;
     }
 
     &::after {
         content: "";
         position: absolute;
-        left: var(--leftOffset);
-        right: 0;
-        height: 1px;
-        bottom: calc(-1 * var(--topOffset));
+        left: calc(var(--leftOffset) + $padding);
+        right: $spacing-24; // 24px: 32px - 8px (right padding)
+        height: $hrHeight;
+        bottom: calc(-1 * var(--topOffset) - $hrHeight); // exclude the height of horizontal line
         background-color: $quinary-content;
         pointer-events: none; // disable the message action bar on hover
     }
@@ -764,22 +763,15 @@ $left-gutter: 64px;
         margin-top: 0;
     }
 
-    padding-top: 0;
-
     .mx_EventTile_avatar {
-        top: 0;
-        left: 0;
+        top: $padding;
+        left: $padding;
     }
 
     .mx_DisambiguatedProfile {
-        margin-left: var(--leftOffset) !important;
-        flex: 1;
         margin-right: 12px;
-
         display: inline-flex;
-        // not a fan of the magic number here, but I just tweaked
-        // the hardcoded value of the current implementation
-        max-width: calc(100% - 96px);
+        flex: 1;
     }
 
     .mx_DisambiguatedProfile_displayName,
@@ -801,14 +793,17 @@ $left-gutter: 64px;
     .mx_EventTile_line {
         width: 100%;
         box-sizing: border-box;
-        padding-left: var(--leftOffset) !important;
         border-radius: $borderRadius !important; // override 4px
     }
 
+    .mx_DisambiguatedProfile,
+    .mx_EventTile_line {
+        padding-inline-start: var(--leftOffset);
+    }
+
     .mx_MessageTimestamp {
-        position: initial !important;
         max-width: 80px;
-        width: auto !important;
+        width: auto;
     }
 }
 
@@ -831,6 +826,8 @@ $left-gutter: 64px;
         flex-direction: column;
 
         .mx_EventTile_line {
+            padding-top: 2px;
+            padding-bottom: 2px;
             padding-left: 0;
             order: 10 !important;
         }
@@ -850,10 +847,10 @@ $left-gutter: 64px;
                 margin-top: 6px; // See: https://github.com/matrix-org/matrix-react-sdk/pull/8442
             }
         }
-    }
 
-    .mx_EventTile:not([data-layout=bubble]) {
-        padding-top: 14px; // due to layout differences, this odd number matches the 18px padding-top of main tl events
+        &:not([data-layout=bubble]) {
+            padding-top: $spacing-16;
+        }
     }
 
     .mx_EventTile[data-layout=bubble] {
@@ -885,7 +882,7 @@ $left-gutter: 64px;
     }
 
     .mx_EventTile[data-layout=group] {
-        $spacing-start: 48px;
+        $spacing-start: 56px; // 56px: 64px - 8px (padding)
         width: 100%;
 
         .mx_EventTile_content,
@@ -916,14 +913,14 @@ $left-gutter: 64px;
         }
 
         .mx_MessageTimestamp {
-            top: 2px !important;
-            width: auto;
+            top: 2px; // Align with mx_EventTile_content
         }
 
         .mx_EventTile_senderDetails {
             display: flex;
             align-items: center;
-            gap: calc(14px + $selected-message-border-width);
+            gap: $spacing-16; // gap between the avatar and the sender ID
+            padding-inline-start: $spacing-8;
 
             a {
                 flex: 1;
@@ -966,4 +963,13 @@ $left-gutter: 64px;
         padding-right: 11px; // align with right edge of input
         margin-right: 0; // align with right edge of background
     }
+
+    .mx_GroupLayout {
+        .mx_EventTile {
+            .mx_EventTile_line {
+                padding-top: 2px;
+                padding-bottom: 2px;
+            }
+        }
+    }
 }
diff --git a/res/css/views/rooms/_ThreadSummary.scss b/res/css/views/rooms/_ThreadSummary.scss
@@ -85,12 +85,8 @@ $threadSummaryLineHeight: calc(2 * $font-12px);
 }
 
 // XXX: these classes are re-used outside of the component
-.mx_ThreadSummary_threads-amount {
-    font-weight: $font-semi-bold;
-    position: relative;
-    padding: 0 $spacing-12 0 $spacing-8;
-    white-space: nowrap;
-    line-height: $threadSummaryLineHeight;
+.mx_ThreadSummary_ThreadsAmount {
+    @mixin ThreadsAmount;
 }
 
 .mx_ThreadSummary_sender {
@@ -113,5 +109,5 @@ $threadSummaryLineHeight: calc(2 * $font-12px);
 }
 
 .mx_ThreadSummary_avatar {
-    margin-right: $spacing-8;
+    margin-inline-end: $spacing-8;
 }
diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx
@@ -517,7 +517,7 @@ export class UnwrappedEventTile extends React.Component<IProps, IState> {
         }
 
         return <div className="mx_ThreadPanel_replies">
-            <span className="mx_ThreadSummary_threads-amount">
+            <span className="mx_ThreadPanel_ThreadsAmount">
                 { this.state.thread.length }
             </span>
             <ThreadMessagePreview thread={this.state.thread} />
@@ -1030,8 +1030,10 @@ export class UnwrappedEventTile extends React.Component<IProps, IState> {
         if (this.context.timelineRenderingType === TimelineRenderingType.Notification) {
             avatarSize = 24;
             needsSenderProfile = true;
-        } else if (this.context.timelineRenderingType === TimelineRenderingType.ThreadsList) {
-            avatarSize = 36;
+        } else if (this.context.timelineRenderingType === TimelineRenderingType.ThreadsList ||
+            (this.context.timelineRenderingType === TimelineRenderingType.Thread && !this.props.continuation)
+        ) {
+            avatarSize = 32;
             needsSenderProfile = true;
         } else if (eventType === EventType.RoomCreate || isBubbleMessage) {
             avatarSize = 0;

diff --git a/src/components/views/rooms/ThreadSummary.tsx b/src/components/views/rooms/ThreadSummary.tsx
@@ -58,7 +58,7 @@ const ThreadSummary = ({ mxEvent, thread }: IProps) => {
             }}
             aria-label={_t("Open thread")}
         >
-            <span className="mx_ThreadSummary_threads-amount">
+            <span className="mx_ThreadSummary_ThreadsAmount">
                 { countSection }
             </span>
             <ThreadMessagePreview thread={thread} showDisplayname={!roomContext.narrow} />