This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 834
Fixes following threads design implementation review #7100
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
cecb764
Change thread summary dimensions
cb2956c
Add gutter for notification dot in thread panel
981aa03
Update thread list spacing and summary
874f494
lint
2221d9c
Hide 'Show Threads' in room info list
6b0db79
Implement chevron on hover for thread summary
63937ea
Fix AutoHideScrollbar implementation in ThreadView
bb65ff6
Thread panel tile displays last reply formatted ts
90be9d2
Add a two line ellipsis for thread panel items
ce87c9c
Fix permalinkCreator on ThreadPanel
d5809f9
Fix thread list context menu not opening
d6475cd
Make thread header button close the right panel
4c39873
Implement context menu dropdown UI
a7f56ee
Implement chevron design
8b77fbe
Fix thread summary sizing
c0ab136
Make thread summary follow parent container sizing
030f087
Fix thread summary UI
0992d40
Apply correct action bar focused state
d709103
Fix e2ee shield position in thread list view
8b4ce47
Fix thread panel header UI
8b6fda9
Update thread copy
36e3bb3
Fix thread summary
8fd611a
design review last
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,21 +18,29 @@ limitations under the License. | |
display: flex; | ||
flex-direction: column; | ||
|
||
padding-right: 0; | ||
|
||
.mx_BaseCard_header { | ||
margin-bottom: 12px; | ||
.mx_BaseCard_close, | ||
.mx_BaseCard_back { | ||
margin-top: 15px; | ||
width: 24px; | ||
height: 24px; | ||
} | ||
.mx_BaseCard_back { | ||
left: -4px; | ||
} | ||
.mx_BaseCard_close { | ||
right: -8px; | ||
right: -4px; | ||
} | ||
} | ||
|
||
.mx_ThreadPanel__header { | ||
.mx_BaseCard_back ~ .mx_ThreadPanel__header { | ||
width: calc(100% - 60px); | ||
margin-left: 30px; | ||
} | ||
|
||
.mx_ThreadPanel__header { | ||
width: calc(100% - 30px); | ||
height: 24px; | ||
display: flex; | ||
flex: 1; | ||
justify-content: space-between; | ||
|
@@ -47,13 +55,23 @@ limitations under the License. | |
|
||
.mx_AccessibleButton { | ||
font-size: 12px; | ||
color: $primary-content; | ||
color: $secondary-content; | ||
} | ||
|
||
.mx_MessageActionBar_optionsButton { | ||
position: relative; | ||
} | ||
|
||
.mx_MessageActionBar_maskButton { | ||
--size: 24px; | ||
width: var(--size); | ||
height: var(--size); | ||
&::after { | ||
mask-size: var(--size); | ||
mask-image: url("$(res)/img/element-icons/message/overflow-large.svg"); | ||
} | ||
} | ||
|
||
.mx_ContextualMenu_wrapper { | ||
// It's added here due to some weird error if I pass it directly in the style, even though it's a numeric value, so it's being passed 0 instead. | ||
// The error: react_devtools_backend.js:2526 Warning: `NaN` is an invalid value for the `top` css style property. | ||
|
@@ -70,38 +88,59 @@ limitations under the License. | |
|
||
font-size: 12px; | ||
color: $secondary-content; | ||
padding-top: 10px; | ||
padding-bottom: 10px; | ||
|
||
border: 1px solid $quinary-content; | ||
box-shadow: 0px 1px 3px rgba(23, 25, 28, 0.05); | ||
} | ||
|
||
.mx_ContextualMenu_chevron_top { | ||
left: auto; | ||
right: 22px; | ||
border-bottom-color: $quinary-content; | ||
&::after { | ||
content: ""; | ||
border: inherit; | ||
border-bottom-color: $background; | ||
position: absolute; | ||
top: 1px; | ||
left: -8px; | ||
} | ||
} | ||
|
||
.mx_ThreadPanel_Header_FilterOptionItem { | ||
display: flex; | ||
flex-grow: 1; | ||
justify-content: space-between; | ||
flex-direction: column; | ||
overflow: visible; | ||
width: 100%; | ||
padding: 20px; | ||
padding-left: 30px; | ||
padding: 10px 20px 10px 30px; | ||
position: relative; | ||
|
||
&:hover { | ||
background-color: $event-selected-color; | ||
} | ||
&[aria-selected="true"] { | ||
&::before { | ||
:first-child { | ||
margin-left: -20px; | ||
} | ||
:first-child::before { | ||
content: ""; | ||
width: 12px; | ||
height: 12px; | ||
grid-column: 1; | ||
grid-row: 1; | ||
margin-right: 8px; | ||
mask-image: url("$(res)/img/feather-customised/check.svg"); | ||
mask-size: 100%; | ||
mask-repeat: no-repeat; | ||
position: absolute; | ||
top: 22px; | ||
left: 10px; | ||
background-color: $primary-content; | ||
display: inline-block; | ||
vertical-align: middle; | ||
} | ||
} | ||
|
||
:last-child { | ||
color: $secondary-content; | ||
} | ||
} | ||
} | ||
|
||
|
@@ -131,24 +170,20 @@ limitations under the License. | |
} | ||
|
||
.mx_AutoHideScrollbar { | ||
border-radius: 8px; | ||
} | ||
|
||
.mx_RoomView_messageListWrapper { | ||
background: #fff; | ||
background-color: $background; | ||
padding: 8px; | ||
border-radius: inherit; | ||
border-radius: 8px; | ||
width: calc(100% - 16px); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It created all sort of funky regressions with the scrollbar. It's likely that a more elegant solution exists but I did not manage to figure it out yet There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as reference: #7105 |
||
padding-right: 16px; | ||
} | ||
|
||
.mx_ScrollPanel { | ||
.mx_RoomView_MessageList { | ||
padding: 0; | ||
} | ||
.mx_RoomView_MessageList { | ||
padding-left: 12px; | ||
padding-right: 0; | ||
} | ||
|
||
.mx_EventTile, .mx_EventListSummary { | ||
// Account for scrollbar when hovering | ||
width: calc(100% - 3px); | ||
margin: 0 2px; | ||
padding-top: 0; | ||
|
||
|
@@ -170,19 +205,28 @@ limitations under the License. | |
.mx_DateSeparator { | ||
display: none; | ||
} | ||
|
||
&.mx_EventTile_clamp:hover { | ||
cursor: pointer; | ||
} | ||
} | ||
|
||
.mx_EventTile:not([data-layout=bubble]) { | ||
.mx_EventTile_e2eIcon { | ||
left: 8px; | ||
} | ||
} | ||
|
||
.mx_MessageComposer { | ||
background-color: $background; | ||
border-radius: 8px; | ||
margin-top: 8px; | ||
width: calc(100% - 8px); | ||
padding: 0 8px; | ||
box-sizing: border-box; | ||
} | ||
|
||
.mx_ThreadPanel_dropdown { | ||
padding: 4px 8px; | ||
padding: 3px 8px; | ||
border-radius: 4px; | ||
line-height: 1.5; | ||
user-select: none; | ||
|
@@ -207,6 +251,36 @@ limitations under the License. | |
.mx_ThreadPanel_dropdown[aria-expanded=true]::before { | ||
transform: rotate(180deg); | ||
} | ||
|
||
.mx_MessageTimestamp { | ||
font-size: $font-12px; | ||
} | ||
} | ||
|
||
.mx_ThreadPanel_replies { | ||
margin-top: 8px; | ||
} | ||
|
||
.mx_ThreadPanel_repliesSummary { | ||
&::before { | ||
content: ""; | ||
mask-image: url('$(res)/img/element-icons/thread-summary.svg'); | ||
mask-position: center; | ||
display: inline-block; | ||
height: 18px; | ||
min-width: 18px; | ||
background-color: currentColor; | ||
mask-repeat: no-repeat; | ||
mask-size: contain; | ||
margin-right: 8px; | ||
vertical-align: middle; | ||
} | ||
|
||
color: $secondary-content; | ||
font-weight: 600; | ||
float: left; | ||
margin-right: 12px; | ||
font-size: $font-12px; | ||
} | ||
|
||
.mx_ThreadPanel_viewInRoom::before { | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
What do you think about this solution? #7105
I also fiddled around with this, when I wanted to get the hidden scrollbar with rounded corners working with the
TimelineCard
(Chat card in the right panel). I wanted it to be based on the same solution as the threads so I played around with the threads as well.Hiding the scrollbar with the browser and not do the margin/padding trick gave good results with rounded corners.
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.
I don't think that hiding the scrollbar is an option. They are an important indicator to orientate yourself in the app, and I believe they serve some accessibility purpose too