Fix timestamp and read receipt group placement on ThreadView #10335
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -170,16 +170,12 @@ limitations under the License. | |
| flex-basis: 48px; /* 12 (padding on message list) + 36 (padding on event lines) */ | ||
| } | ||
|
|
||
| .mx_GenericEventListSummary_unstyledList, | ||
| .mx_RoomView_MessageList { | ||
| .mx_EventTile[data-layout="bubble"] { | ||
| /* RR next to a message, in either an event list summary or the message list */ | ||
| .mx_ReadReceiptGroup { | ||
|
There was a problem hiding this comment. It looks that the magic number should in fact have been |
||
| inset-inline-end: var(--BaseCard_EventTile_bubble_RR-inset-inline-end); | ||
|
There was a problem hiding this comment. There was a problem hiding this comment. Once more, just linking to a diff isn't very helpful. In this case, this is a link to the other place that |
||
| } | ||
|
|
||
| &.mx_EventTile_info { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1032,77 +1032,106 @@ $left-gutter: 64px; | |
| --ThreadView_group_spacing-end: 8px; /* same as padding */ | ||
|
|
||
| .mx_EventTile { | ||
|
Comment on lines
-1035
to
-1036
There was a problem hiding this comment. why is it safe to remove these? |
||
| .mx_EventTile_roomName { | ||
| display: none; | ||
| } | ||
|
|
||
| /* handling for hidden events (e.g reactions) in the thread view */ | ||
| &.mx_EventTile_info { | ||
| .mx_EventTile_avatar { | ||
| margin-block: 0; | ||
|
Comment on lines
-1047
to
+1042
There was a problem hiding this comment. I have to say, I find the separate |
||
| width: 14px; /* avatar img size */ | ||
| height: 14px; /* avatar img size */ | ||
| } | ||
|
|
||
|
Comment on lines
-1053
to
-1057
|
||
| &.mx_EventTile_selected .mx_EventTile_line, | ||
| .mx_EventTile_line { | ||
| $line-height: $font-12px; | ||
| line-height: $line-height; | ||
|
|
||
| /* hidden event content */ | ||
| .mx_EventTile_content, | ||
| /* redacted hidden event */ | ||
| .mx_RedactedBody { | ||
| font-size: $line-height; | ||
| } | ||
| } | ||
|
|
||
| .mx_ViewSourceEvent_toggle { | ||
| display: none; /* hide the hidden event expand button, not enough | ||
| space, view source can still be used */ | ||
| } | ||
|
|
||
| /* Required for override */ | ||
| &[data-layout="irc"], | ||
| &[data-layout="group"], | ||
| &[data-layout="bubble"] { | ||
| /* Override | ||
| .mx_ThreadView .mx_EventTile[data-layout="group"], | ||
| .mx_GenericEventListSummary[data-layout="bubble"][data-expanded="true"] .mx_EventTile_info */ | ||
| padding-block: 0; | ||
|
|
||
| /* Add padding to hidden event line of all of the layouts, including bubble layout */ | ||
| .mx_EventTile_line { | ||
| padding-block: var(--BaseCard_EventTile_line-padding-block); | ||
| } | ||
| } | ||
|
|
||
| &[data-layout="irc"], | ||
| &[data-layout="group"] { | ||
| .mx_EventTile_avatar { | ||
| position: absolute; | ||
| top: 1.5px; /* Align with hidden event content */ | ||
| left: calc($MessageTimestamp_width + 14px - 4px); /* 14px: avatar width, 4px: align with text */ | ||
| z-index: 9; /* position above the hover styling */ | ||
| } | ||
|
|
||
| &.mx_EventTile_selected .mx_EventTile_line, | ||
| .mx_EventTile_line { | ||
| .mx_EventTile_content, | ||
| .mx_RedactedBody { | ||
| /* Add space between avatar and hidden event content */ | ||
| /* 14px: avatar width, 6px: 20px - 14px */ | ||
| margin-inline-start: calc(14px + 6px); | ||
| } | ||
| } | ||
|
|
||
| &.mx_EventTile_selected .mx_EventTile_line { | ||
| /* Override | ||
| .mx_EventTile[data-layout="group"].mx_EventTile_info.mx_EventTile_selected .mx_EventTile_line */ | ||
| padding-inline-start: var(--ThreadView_group_spacing-start); | ||
| } | ||
|
|
||
| .mx_MessageTimestamp { | ||
| top: 2px; /* Align with avatar */ | ||
| } | ||
|
|
||
| .mx_ReadReceiptGroup { | ||
| inset-block-start: calc(-$font-22px); /* Align with hidden event content */ | ||
| } | ||
|
|
||
| &:hover { | ||
| &.mx_EventTile_verified.mx_EventTile_info .mx_EventTile_line, | ||
| &.mx_EventTile_unverified.mx_EventTile_info .mx_EventTile_line, | ||
| &.mx_EventTile_unknown.mx_EventTile_info .mx_EventTile_line { | ||
| padding-inline-start: var(--ThreadView_group_spacing-start); /* Override */ | ||
| } | ||
| } | ||
| } | ||
|
|
||
| &[data-layout="bubble"] { | ||
| /* Same declaration as the main timeline */ | ||
| margin-inline: var(--EventTile_bubble-margin-inline-start) 0; | ||
|
|
||
| /* Shadow on hover */ | ||
| &::before { | ||
| /* Apply the same value as the main timeline */ | ||
| inset-inline: calc(-1 * var(--EventTile_bubble-margin-inline-start)) 0; | ||
| } | ||
|
|
||
| .mx_ReadReceiptGroup { | ||
| /* Move read receipt group inside ThreadView */ | ||
| inset-inline-end: 0; | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -1172,16 +1201,18 @@ $left-gutter: 64px; | |
| &.mx_EventTile_verified.mx_EventTile_info .mx_EventTile_line, | ||
| &.mx_EventTile_unverified.mx_EventTile_info .mx_EventTile_line, | ||
| &.mx_EventTile_unknown.mx_EventTile_info .mx_EventTile_line { | ||
| padding-inline-start: 0; /* TODO: investigate where this is used */ | ||
| } | ||
| } | ||
| } | ||
|
|
||
| &[data-layout="bubble"] { | ||
| margin-inline-start: var(--BaseCard_EventTile-spacing-inline); | ||
| margin-inline-end: var(--EventTile_bubble-margin-inline-end); /* Reserve space for read receipt group */ | ||
|
|
||
| &::before { | ||
| inset-inline-start: calc(-1 * var(--BaseCard_EventTile-spacing-inline)); | ||
| inset-inline-end: calc(-1 * var(--EventTile_bubble-margin-inline-end)); | ||
|
There was a problem hiding this comment. There was a problem hiding this comment. please don't just link to another line in the diff. What am I supposed to learn by looking at that link? |
||
| z-index: auto; /* enable background color on hover */ | ||
| } | ||
|
|
||
|
|
@@ -1195,9 +1226,11 @@ $left-gutter: 64px; | |
| max-width: var(--EventBubbleTile_line-max-width); | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. There was a problem hiding this comment. again, what do I learn by looking at that link? |
||
| .mx_ReadReceiptGroup { | ||
| inset-inline-end: var(--BaseCard_EventTile_bubble_RR-inset-inline-end); | ||
| } | ||
|
|
||
| &[data-self="true"] { | ||
| .mx_EventTile_line.mx_EventTile_mediaLine { | ||
| margin: 0 var(--EventTile_bubble_line-margin-inline-end) 0 0; /* align with normal messages */ | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,9 +14,12 @@ See the License for the specific language governing permissions and | |||||||
| limitations under the License. | ||||||||
| */ | ||||||||
|
|
||||||||
| /* Required due to alphabetic order of CSS import */ | ||||||||
|
There was a problem hiding this comment. we do this elsewhere without a comment; I don't think the comment here is that helpful.
Suggested change
|
||||||||
| :root { | ||||||||
|
There was a problem hiding this comment. Otherwise this variable could not be used on |
||||||||
| --ReadReceiptGroup_EventBubbleTile-spacing-end: 78px; | ||||||||
|
There was a problem hiding this comment. it would be good to add a comment saying what this variable does though. I'm struggling to figure it out. The best definition I came up with was:
Suggested change
Any better ideas? (why isn't it just -78px, to save us doing |
||||||||
| } | ||||||||
|
|
||||||||
| .mx_ReadReceiptGroup { | ||||||||
| position: relative; | ||||||||
| display: inline-block; | ||||||||
| user-select: none; | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1197,8 +1197,8 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState> | |
| <a href={permalink} onClick={this.onPermalinkClicked}> | ||
| {timestamp} | ||
| </a> | ||
| </div>, | ||
| msgOption, | ||
|
There was a problem hiding this comment. See: https://github.com/matrix-org/matrix-react-sdk/pull/10335/files#diff-9ff5535291b7c2e655ccd3d02c4f632b6f2dc9c30072514641e31c2a8751c57eR1287 and https://github.com/matrix-org/matrix-react-sdk/pull/10335/files#diff-9ff5535291b7c2e655ccd3d02c4f632b6f2dc9c30072514641e31c2a8751c57eR1398. Both of them are placed outside of There was a problem hiding this comment. I didn't understand what these links were supposed to tell me. For the benefit of future reviewers: This brings |
||
| reactionsRow, | ||
| ], | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/matrix-org/matrix-react-sdk/pull/10335/files#diff-5d3e25eb1d22dd7a96e0e87ea9c1298dc7dbce8eb965d30b2d1ebc1436bfec3fR1108