Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Align message context menu to right and vertically where space available #3087

Merged
merged 3 commits into from
Jun 12, 2019

Conversation

jryans
Copy link
Collaborator

@jryans jryans commented Jun 10, 2019

This aligns the message context menu to the right edge of the menu button that triggers it and vertically places it either on the top or bottom of the button depending on which side has more space available.

Message near bottom of timeline:

2019-06-10 at 16 33

Message near top of timeline:

2019-06-10 at 16 34

Fixes element-hq/element-web#9624

jryans added 3 commits June 10, 2019 16:07
For context menus without chevrons, this changes the menu components to still
set default styles that align the menu based on the edges used to specify the
menu's position. This is not intended to change the positioning of any existing
menus.
This changes the message context menu to align the right edge of the menu with
the right edge of the button that opens it, which should keep all menu options
inside the viewport, even if they are very wide.

Part of element-hq/element-web#9624
This aligns the message context menu on either the top or the bottom of the
button that triggers, depending on which side has more space available to fit
the menu.

Fixes element-hq/element-web#9624
@jryans jryans requested a review from a team June 10, 2019 15:35
@turt2live
Copy link
Member

ooi how much of element-hq/element-web#3429 is fixed? (noting that this PR would be the 3rd PR in the series)

@jryans
Copy link
Collaborator Author

jryans commented Jun 11, 2019

ooi how much of vector-im/riot-web#3429 is fixed? (noting that this PR would be the 3rd PR in the series)

This change is (mostly) specific to the message context menu only. (It does also add some helper features to the context menu component as well, but we'd still need to visit and tweak the other cases.)

Nad's design for this case seems specific to this particular menu, so I didn't feel confident about using that more generally for the other ones in element-hq/element-web#3429.

So anyway, it does fix one comment from that issue, but the others (maybe only key share tooltip...?) are left as separate work.

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

I'm worried that the chevron will drift when zooming for the other popovers (stickers, etc), but this looks fine.

I feel like overall this entire component is a bit complicated though, but that's a problem for another day.

@jryans
Copy link
Collaborator Author

jryans commented Jun 12, 2019

I'm worried that the chevron will drift when zooming for the other popovers (stickers, etc), but this looks fine.

I tried to check the chevrons for the other context menus / tooltips, so hopefully they are okay.

I feel like overall this entire component is a bit complicated though, but that's a problem for another day.

Yes, it is a bit complex / over-engineered.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Aggregations: Avoid clipping context menu
2 participants