Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(Chat): properly position actions with overflow #2363

Merged
merged 8 commits into from
Feb 20, 2020
Merged

fix(Chat): properly position actions with overflow #2363

merged 8 commits into from
Feb 20, 2020

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Feb 17, 2020 •
edited

Fixes #2298.

@layershifter layershifter added 🚧 WIP 🧰 fix Introduces fix for broken behavior. labels Feb 17, 2020
@DustyTheBot
Copy link
Collaborator

DustyTheBot commented Feb 17, 2020 •
edited

Perf comparison

Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.48 0.41 1.17:1 2000 959
🦄 Button.Fluent 0.11 0.18 0.61:1 1000 111
🔧 Checkbox.Fluent 0.79 0.28 2.82:1 1000 789
🔧 Dialog.Fluent 0.37 0.19 1.95:1 5000 1833
🔧 Dropdown.Fluent 3.62 0.4 9.05:1 1000 3618
🔧 Icon.Fluent 0.13 0.03 4.33:1 5000 643
🦄 Image.Fluent 0.05 0.08 0.63:1 5000 234
🔧 Slider.Fluent 1.47 0.28 5.25:1 1000 1471
🔧 Text.Fluent 0.05 0.02 2.5:1 5000 257
🦄 Tooltip.Fluent 0.11 18.25 0.01:1 5000 528

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ProviderMergeThemesPerf.default 1451 1148 1.26:1
SegmentMinimalPerf.default 1520 1266 1.2:1
Icon.Fluent 643 560 1.15:1
BoxMinimalPerf.default 260 231 1.13:1
ImageMinimalPerf.default 244 215 1.13:1
RadioGroupMinimalPerf.default 466 417 1.12:1
HeaderMinimalPerf.default 455 410 1.11:1
AccordionMinimalPerf.default 211 192 1.1:1
FlexMinimalPerf.default 365 336 1.09:1
ItemLayoutMinimalPerf.default 1776 1631 1.09:1
PopupMinimalPerf.default 414 379 1.09:1
ToolbarMinimalPerf.default 957 879 1.09:1
AlertMinimalPerf.default 572 532 1.08:1
TextAreaMinimalPerf.default 3108 2869 1.08:1
Dropdown.Fluent 3618 3376 1.07:1
DialogMinimalPerf.default 1667 1571 1.06:1
HeaderSlotsPerf.default 1369 1287 1.06:1
ListWith60ListItems.default 152 145 1.05:1
ChatDuplicateMessagesPerf.default 352 337 1.04:1
DropdownManyItemsPerf.default 413 396 1.04:1
FormMinimalPerf.default 749 722 1.04:1
Image.Fluent 234 224 1.04:1
Slider.Fluent 1471 1416 1.04:1
SplitButtonMinimalPerf.default 13229 12887 1.03:1
Text.Fluent 257 249 1.03:1
DividerMinimalPerf.default 890 869 1.02:1
SliderMinimalPerf.default 1578 1551 1.02:1
AnimationMinimalPerf.default 461 455 1.01:1
DropdownMinimalPerf.default 3366 3342 1.01:1
MenuMinimalPerf.default 1848 1833 1.01:1
RefMinimalPerf.default 158 156 1.01:1
Avatar.Fluent 959 948 1.01:1
Button.Fluent 111 110 1.01:1
CarouselMinimalPerf.default 1889 1896 1:1
ChatMinimalPerf.default 407 409 1:1
ListNestedPerf.default 668 668 1:1
Dialog.Fluent 1833 1841 1:1
AvatarMinimalPerf.default 531 537 0.99:1
HierarchicalTreeMinimalPerf.default 761 765 0.99:1
AttachmentSlotsPerf.default 3255 3317 0.98:1
ButtonSlotsPerf.default 570 580 0.98:1
GridMinimalPerf.default 776 793 0.98:1
ListMinimalPerf.default 312 320 0.98:1
LoaderMinimalPerf.default 2351 2392 0.98:1
ReactionMinimalPerf.default 2684 2737 0.98:1
CheckboxMinimalPerf.default 3522 3621 0.97:1
EmbedMinimalPerf.default 5948 6125 0.97:1
TableMinimalPerf.default 570 589 0.97:1
TextMinimalPerf.default 271 280 0.97:1
LayoutMinimalPerf.default 491 510 0.96:1
CustomToolbarPrototype.default 3610 3774 0.96:1
TreeWith60ListItems.default 233 243 0.96:1
AttachmentMinimalPerf.default 877 925 0.95:1
InputMinimalPerf.default 874 919 0.95:1
ListCommonPerf.default 698 736 0.95:1
LabelMinimalPerf.default 825 874 0.94:1
ProviderMinimalPerf.default 664 707 0.94:1
TreeMinimalPerf.default 849 903 0.94:1
Tooltip.Fluent 528 563 0.94:1
ButtonMinimalPerf.default 112 122 0.92:1
ChatWithPopoverPerf.default 487 532 0.92:1
MenuButtonMinimalPerf.default 1413 1529 0.92:1
TooltipMinimalPerf.default 702 762 0.92:1
StatusMinimalPerf.default 236 263 0.9:1
VideoMinimalPerf.default 756 879 0.86:1
IconMinimalPerf.default 292 346 0.84:1
Checkbox.Fluent 789 939 0.84:1
PortalMinimalPerf.default 241 335 0.72:1

Generated by 🚫 dangerJS

layershifter
layershifter commented Feb 17, 2020
View reviewed changes
packages/react/src/components/Chat/ChatMessage.tsx
height: 0,
}
const messageRect: DOMRect = (positionActionMenu &&
_.invoke(messageNode, 'getBoundingClientRect')) || { height: 0 }
Copy link
Member Author

Choose a reason for hiding this comment

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

When we paired with @miroslavstastny we realized that messageRect.height is enough in this case

@layershifter layershifter merged commit 705bdef into master Feb 20, 2020
@layershifter layershifter deleted the fix/chat-overflow branch February 20, 2020 16:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@miroslavstastny miroslavstastny miroslavstastny approved these changes

@dvdzkwsk dvdzkwsk Awaiting requested review from dvdzkwsk

@dzearing dzearing Awaiting requested review from dzearing dzearing is a code owner

@ecraig12345 ecraig12345 Awaiting requested review from ecraig12345 ecraig12345 is a code owner

@JasonGore JasonGore Awaiting requested review from JasonGore JasonGore is a code owner

@jdhuntington jdhuntington Awaiting requested review from jdhuntington

@kenotron kenotron Awaiting requested review from kenotron kenotron is a code owner

@levithomason levithomason Awaiting requested review from levithomason levithomason is a code owner

@mnajdova mnajdova Awaiting requested review from mnajdova

Assignees
No one assigned
Labels
🧰 fix Introduces fix for broken behavior. 🚀 ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Chat: unstable_overflow is broken in ChatMessage
3 participants
@layershifter @DustyTheBot @miroslavstastny