Skip to content

Wmwragg/one to one chat#2110

Merged
ara4n merged 39 commits intodevelopfrom
wmwragg/one-to-one-chat
Sep 9, 2016
Merged

Wmwragg/one to one chat#2110
ara4n merged 39 commits intodevelopfrom
wmwragg/one-to-one-chat

Conversation

@wmwragg
Copy link
Copy Markdown
Contributor

@wmwragg wmwragg commented Sep 7, 2016

This should close #1882 . This also adds and fixes the following:

  • LHS Panel (collapsed state) now has notification badges in it's headers
  • LHS Panel (collapsed state) now has long hover on header to show the header label and room count
  • Fixed Regression: Dialog buttons, now use the correct font and font size
  • Fixed Bug: In Safari when using "Show scroll bars: Always", the tooltips were hidden, this no longer happens
  • Fixed Bug: Tooltips could be offset from their correct position, they shouldn't be now
  • Fixed Bug: Tooltips would be incorrectly positioned when zoomed in, they shouldn't be now
  • Fixed Bug: When zoomed in the LHS Panel sticky headers weren't being correctly positioned, they should be now
  • Tooltips now have the correct styling
  • Tooltips should now appear next to the LHS Panel bottom row buttons

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 the ChatInviteDialog 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

…ixels in size between textarea and AddressTile
@matrixbot
Copy link
Copy Markdown

Can one of the admins verify this patch?

if (show) {
var RoomTooltip = sdk.getComponent("rooms.RoomTooltip");
return <RoomTooltip name={name}/>;
return <RoomTooltip label={label} left={6} top={this.props.collapsed ? 3 : null} />;
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Contributor Author

@wmwragg wmwragg Sep 8, 2016

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

Copy link
Copy Markdown
Contributor Author

@wmwragg wmwragg Sep 8, 2016

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

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Member

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!

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

@wmwragg wmwragg Sep 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idea worked, committed

@ara4n
Copy link
Copy Markdown
Member

ara4n commented Sep 8, 2016

lgtm modulo minor niggle. will merge in the morning.

@ara4n ara4n merged commit 8376f0d into develop Sep 9, 2016
@t3chguy t3chguy deleted the wmwragg/one-to-one-chat branch May 12, 2022 08:55
t3chguy pushed a commit that referenced this pull request Mar 25, 2026
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement new "talk to a person" UI

3 participants