From 0bf5d54041cb3a6cac03d1b9e4e1a2212d1cf2c5 Mon Sep 17 00:00:00 2001 From: Suguru Hirahara Date: Mon, 4 Jul 2022 19:07:50 +0000 Subject: [PATCH] Improve style rules for thread summary (#8868) * Use mixin ThreadSummaryIcon Signed-off-by: Suguru Hirahara * Tidy mx_ThreadSummary Signed-off-by: Suguru Hirahara * Move style blocks from _EventTile.scss to _ThreadSummary.scss Signed-off-by: Suguru Hirahara * Merge mx_ThreadSummaryIcon::before Signed-off-by: Suguru Hirahara * From threads amount to replies amount Signed-off-by: Suguru Hirahara * Remove obsolete declaration and class names mixin ThreadSummaryIcon has "background-color: $secondary-content !important" Signed-off-by: Suguru Hirahara * Move mx_ThreadPanel_replies::before from _ThreadSummary to _EventTile.scss Signed-off-by: Suguru Hirahara * Rename mx_ThreadSummaryIcon to mx_ThreadSummary_icon Signed-off-by: Suguru Hirahara * Use variables and remove obsolete one Signed-off-by: Suguru Hirahara * Merge style rules, renaming a variable Signed-off-by: Suguru Hirahara * Include mx_MessagePanel_narrow in mx_ThreadSummary Signed-off-by: Suguru Hirahara * Remove a redundant declaration Signed-off-by: Suguru Hirahara * Use a variable Signed-off-by: Suguru Hirahara * Include mx_ThreadSummary_sender and mx_ThreadSummary_content in mx_ThreadSummary Expected according to tests Signed-off-by: Suguru Hirahara * Remove a variable used only once Signed-off-by: Suguru Hirahara * Ensure the same line-height is applied Signed-off-by: Suguru Hirahara * Remove !important Signed-off-by: Suguru Hirahara --- res/css/_common.scss | 11 +-- res/css/views/right_panel/_ThreadPanel.scss | 8 +- res/css/views/rooms/_EventTile.scss | 40 ++------ res/css/views/rooms/_ThreadSummary.scss | 97 +++++++++++++------- src/components/views/rooms/EventTile.tsx | 4 +- src/components/views/rooms/ThreadSummary.tsx | 2 +- 6 files changed, 82 insertions(+), 80 deletions(-) diff --git a/res/css/_common.scss b/res/css/_common.scss index 1e8daf3ba3..e04bf616f9 100644 --- a/res/css/_common.scss +++ b/res/css/_common.scss @@ -775,25 +775,22 @@ legend { } } -@define-mixin ThreadsAmount { - $threadInfoLineHeight: calc(2 * $font-12px); - +@define-mixin ThreadRepliesAmount { 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 { +@define-mixin ThreadSummaryIcon { content: ""; display: inline-block; mask-image: url('$(res)/img/element-icons/thread-summary.svg'); mask-position: center; + mask-repeat: no-repeat; + mask-size: contain; height: 18px; min-width: 18px; background-color: $secondary-content !important; - mask-repeat: no-repeat; - mask-size: contain; } diff --git a/res/css/views/right_panel/_ThreadPanel.scss b/res/css/views/right_panel/_ThreadPanel.scss index ca0b4409ca..d83b51272e 100644 --- a/res/css/views/right_panel/_ThreadPanel.scss +++ b/res/css/views/right_panel/_ThreadPanel.scss @@ -208,15 +208,9 @@ limitations under the License. border-radius: 50%; &::after { - content: ""; + @mixin ThreadSummaryIcon; width: inherit; height: inherit; - mask-image: url('$(res)/img/element-icons/thread-summary.svg'); - mask-position: center; - display: inline-block; - mask-repeat: no-repeat; - mask-size: contain; - background-color: $secondary-content; } } diff --git a/res/css/views/rooms/_EventTile.scss b/res/css/views/rooms/_EventTile.scss index 29968ed41f..89f25f5d61 100644 --- a/res/css/views/rooms/_EventTile.scss +++ b/res/css/views/rooms/_EventTile.scss @@ -16,7 +16,6 @@ limitations under the License. */ $left-gutter: 64px; -$threadInfoLineHeight: calc(2 * $font-12px); // See: _commons.scss .mx_EventTile { --EventTile_content-margin-inline-end: 34px; // TODO: Use a spacing variable @@ -24,6 +23,7 @@ $threadInfoLineHeight: calc(2 * $font-12px); // See: _commons.scss --EventTile_group_line-spacing-block-end: 3px; --EventTile_group_line-spacing-inline-start: $left-gutter; --EventTile_group_line-line-height: $font-22px; + --EventTile_ThreadSummary-line-height: calc(2 * $font-12px); flex-shrink: 0; @@ -204,7 +204,7 @@ $threadInfoLineHeight: calc(2 * $font-12px); // See: _commons.scss } .mx_ThreadSummary, - .mx_ThreadSummaryIcon { + .mx_ThreadSummary_icon { margin-left: $left-gutter; } @@ -484,7 +484,7 @@ $threadInfoLineHeight: calc(2 * $font-12px); // See: _commons.scss .mx_EventTile[data-layout=group] { .mx_ThreadSummary, - .mx_ThreadSummaryIcon, + .mx_ThreadSummary_icon, .mx_EventTile_line { /* ideally should be 100px, but 95px gives us a max thumbnail size of 800x600, which is nice */ margin-right: 80px; @@ -722,33 +722,6 @@ $threadInfoLineHeight: calc(2 * $font-12px); // See: _commons.scss } } -.mx_ThreadPanel_replies::before, -.mx_ThreadSummaryIcon::before, -.mx_ThreadSummary::before { - @mixin ThreadInfoIcon; - background-color: $secondary-content !important; -} - -.mx_ThreadSummaryIcon { - display: inline-block; - font-size: $font-12px; - color: $secondary-content !important; - margin-top: 8px; - margin-bottom: 8px; - - &::before { - vertical-align: middle; - margin-right: 8px; - margin-top: -2px; - } -} - -.mx_MessagePanel_narrow .mx_ThreadSummary { - min-width: initial; - max-width: 100%; // prevent overflow - width: initial; -} - .mx_EventTile[data-shape=ThreadsList] { --topOffset: $spacing-12; --leftOffset: 48px; @@ -864,8 +837,13 @@ $threadInfoLineHeight: calc(2 * $font-12px); // See: _commons.scss align-items: center; position: relative; + &::before { + @mixin ThreadSummaryIcon; + } + .mx_ThreadPanel_replies_amount { - @mixin ThreadsAmount; + @mixin ThreadRepliesAmount; + line-height: var(--EventTile_ThreadSummary-line-height); font-size: $font-12px; // Same font size as the counter on the main panel } } diff --git a/res/css/views/rooms/_ThreadSummary.scss b/res/css/views/rooms/_ThreadSummary.scss index 11ff6cdbbe..a1821c5a59 100644 --- a/res/css/views/rooms/_ThreadSummary.scss +++ b/res/css/views/rooms/_ThreadSummary.scss @@ -14,26 +14,35 @@ See the License for the specific language governing permissions and limitations under the License. */ -$left-gutter: 64px; // From _EventTile.scss -$threadSummaryLineHeight: calc(2 * $font-12px); +.mx_ThreadSummary, +.mx_ThreadSummary_content, +.mx_ThreadSummary_icon { + font-size: $font-12px; +} + +.mx_ThreadSummary, +.mx_ThreadSummary_content { + color: $secondary-content; +} + +.mx_ThreadSummary, +.mx_ThreadSummary_icon { + margin-top: $spacing-8; +} .mx_ThreadSummary { min-width: 267px; - max-width: min(calc(100% - $left-gutter), 600px); // leave space on both left & right gutters + max-width: min(calc(100% - var(--EventTile_group_line-spacing-inline-start)), 600px); // leave space on both left & right gutters width: fit-content; height: 40px; position: relative; background-color: $system; - padding-left: $spacing-12; + padding-inline: $spacing-12 $spacing-16; display: flex; align-items: center; - border-radius: 8px; - padding-right: $spacing-16; - margin-top: $spacing-8; - font-size: $font-12px; - color: $secondary-content; - box-sizing: border-box; justify-content: flex-start; + border-radius: 8px; + box-sizing: border-box; clear: both; overflow: hidden; border: 1px solid $system; // always render a border so the hover effect doesn't require a re-layout @@ -70,7 +79,6 @@ $threadSummaryLineHeight: calc(2 * $font-12px); &:hover, &:focus { - cursor: pointer; border-color: $quinary-content; .mx_ThreadSummary_chevron { @@ -80,34 +88,59 @@ $threadSummaryLineHeight: calc(2 * $font-12px); } &::before { + @mixin ThreadSummaryIcon; align-self: center; // v-align the threads icon } -} -// XXX: these classes are re-used outside of the component -.mx_ThreadSummary_ThreadsAmount { - @mixin ThreadsAmount; -} + .mx_ThreadSummary_sender, + .mx_ThreadSummary_content, + .mx_ThreadSummary_replies_amount { + line-height: var(--EventTile_ThreadSummary-line-height); + } -.mx_ThreadSummary_sender { - font-weight: $font-semi-bold; - line-height: $threadSummaryLineHeight; - text-overflow: ellipsis; - overflow: hidden; - white-space: nowrap; -} + .mx_ThreadSummary_sender, + .mx_ThreadSummary_content { + text-overflow: ellipsis; + overflow: hidden; + white-space: nowrap; + } -.mx_ThreadSummary_content { - text-overflow: ellipsis; - overflow: hidden; - white-space: nowrap; - margin-left: $spacing-4; - font-size: $font-12px; - line-height: $threadSummaryLineHeight; - color: $secondary-content; - flex: 1; + .mx_ThreadSummary_sender { + font-weight: $font-semi-bold; + } + + .mx_ThreadSummary_content { + margin-left: $spacing-4; + flex: 1; + } + + .mx_ThreadSummary_replies_amount { + @mixin ThreadRepliesAmount; + } + + .mx_MessagePanel_narrow & { + min-width: initial; + max-width: 100%; // prevent overflow + width: initial; + } } .mx_ThreadSummary_avatar { margin-inline-end: $spacing-8; } + +.mx_ThreadSummary_icon { + display: inline-block; + margin-bottom: $spacing-8; + + &::before { + @mixin ThreadSummaryIcon; + vertical-align: middle; + margin-inline-end: $spacing-8; + margin-top: -2px; + } + + a& { + color: $secondary-content; + } +} diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index bd2d7fb961..3c5e419d94 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -533,14 +533,14 @@ export class UnwrappedEventTile extends React.Component { if (this.context.timelineRenderingType === TimelineRenderingType.Search && this.props.mxEvent.threadRootId) { if (this.props.highlightLink) { return ( - + { _t("From a thread") } ); } return ( -

{ _t("From a thread") }

+

{ _t("From a thread") }

); } } diff --git a/src/components/views/rooms/ThreadSummary.tsx b/src/components/views/rooms/ThreadSummary.tsx index 56ed1f9ff8..968727022f 100644 --- 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")} > - + { countSection }