-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wmwragg/one to one chat #2110
Wmwragg/one to one chat #2110
Conversation
…n a React component
…her than just appearing on hover
…ill used in the RightPanel
…ixels in size between textarea and AddressTile
Can one of the admins verify this patch? |
var RoomTooltip = sdk.getComponent("rooms.RoomTooltip"); | ||
return <RoomTooltip name={name}/>; | ||
return <RoomTooltip label={label} left={6} top={this.props.collapsed ? 3 : null} />; |
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.
in general we've tried to keep all the horrible magic constants hidden in CSS, so slightly unfortunate to see them here... can this not be done with CSS?
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.
Yeah, I didn't like doing it, but they have to be position fixed to float outside the LHS Panel, and to make that nice and simple, the position is calculated within the Tooltip code. I'll have a think and see if I can come up with something better
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.
After an hour and a half, I say keep as is I'm afraid. It can be done, but the result is even more complicated and error prone than passing fixed tweak values to the tooltip, as both calculation and CSS rules compete in highly unintuitive ways. These values have always been here, they were just buried in the MatrixChat reposition code, I've just made them more explicit
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've just had a thought that might work, I'll try it tomorrow morning
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.
honestly it's not worth burning hours on; was just a minor niggle as per the comment. happy to ignore it if it's a pain!
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.
The idea I had should only take a few minutes to try, and it'll either work or it won't, so I'm happy to give it a go
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.
Idea worked, committed
lgtm modulo minor niggle. will merge in the morning. |
…her than passed in as parameters
This should close #1882 . This also adds and fixes the following:
On the address list, you can use the arrow keys to scroll up and down, and return to select. You can also use the mouse to scroll up and down, and use click to select. When using the mouse the address under the mouse is selected as the list is scrolled, you can still hit return to select it.
I have capped the list at 40, but this can be any value you wish, it's set in the
TRUNCATE_QUERY_LIST
constant at the top of theChatInviteDialog
component.A companion branch matrix-org/matrix-react-sdk:wmwragg/one-to-one-chat also needs to be merged.
Signed-off-by: William Wragg wm.wragg@gmail.com