fix(Popup): prevent closing popup when finishing select text outside by chpalac · Pull Request #19201 · microsoft/fluentui

chpalac · 2021-07-30T11:49:56Z

Pull request checklist

Addresses an existing issue: Fixes #0000
Include a change request file using $ yarn change

Description of changes

Starting selecting text inside popup and finishing it outside will trigger click event and consider a click outside, therefore closing the popup. That happens because click event fires after the mousedown and mouseup events.

To fix that I am replacing the document event listener from click to mousedown achieving the desired behavior of keeping the popup open in that situation.

Focus areas to test

(optional)

DustyTheBot · 2021-07-30T11:52:19Z

	Warnings
⚠️	There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Generated by 🚫 dangerJS against 83a7995

codesandbox-ci · 2021-07-30T11:58:48Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 83a7995:

Sandbox	Source
Fluent UI React Starter	Configuration

size-auditor · 2021-07-30T12:00:51Z

Asset size changes

Project	Bundle	Baseline Size	New Size	Difference
office-ui-fabric-react	fluentui-react-northstar-SplitButton	190.115 kB	190.28 kB	165 bytes
office-ui-fabric-react	fluentui-react-northstar-Toolbar	186.907 kB	187.072 kB	165 bytes
office-ui-fabric-react	fluentui-react-northstar-Datepicker	199.162 kB	199.327 kB	165 bytes
office-ui-fabric-react	fluentui-react-northstar-Popup	144.917 kB	145.081 kB	164 bytes
office-ui-fabric-react	fluentui-react-northstar-MenuButton	174.846 kB	175.01 kB	164 bytes

Over Tolerance (1024 B) Over Baseline Below Baseline New Removed 1 kB = 1000 B

Baseline commit: 86476ee0511ad2693c2829b959f93a87ad10f095 (build)

fabricteam · 2021-07-30T12:04:18Z

📊 Bundle size report

_{🤖 This report was generated against 86476ee0511ad2693c2829b959f93a87ad10f095}

fabricteam · 2021-07-30T12:20:34Z

Perf Analysis (`@fluentui/react`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	792	845	5000
BaseButton	mount	888	934	5000
Breadcrumb	mount	2669	2624	1000
ButtonNext	mount	432	418	5000
Checkbox	mount	1524	1563	5000
CheckboxBase	mount	1290	1283	5000
ChoiceGroup	mount	4759	4804	5000
ComboBox	mount	1014	969	1000
CommandBar	mount	10377	10163	1000
ContextualMenu	mount	6322	6309	1000
DefaultButton	mount	1114	1112	5000
DetailsRow	mount	3743	3741	5000
DetailsRowFast	mount	3734	3711	5000
DetailsRowNoStyles	mount	3587	3550	5000
Dialog	mount	2149	2121	1000
DocumentCardTitle	mount	146	142	1000
Dropdown	mount	3310	3224	5000
FluentProviderNext	mount	7567	7471	5000
FocusTrapZone	mount	1794	1830	5000
FocusZone	mount	1803	1845	5000
IconButton	mount	1768	1727	5000
Label	mount	340	331	5000
Layer	mount	1800	1762	5000
Link	mount	465	461	5000
MakeStyles	mount	1864	1830	50000
MenuButton	mount	1443	1475	5000
MessageBar	mount	2146	2138	5000
Nav	mount	3261	3311	1000
OverflowSet	mount	1061	1037	5000
Panel	mount	1359	2120	1000
Persona	mount	810	832	1000
Pivot	mount	1405	1427	1000
PrimaryButton	mount	1289	1262	5000
Rating	mount	7780	7634	5000
SearchBox	mount	1370	1343	5000
Shimmer	mount	2563	2568	5000
Slider	mount	1958	2004	5000
SpinButton	mount	5166	5018	5000
Spinner	mount	417	434	5000
SplitButton	mount	3257	3137	5000
Stack	mount	498	509	5000
StackWithIntrinsicChildren	mount	1533	1496	5000
StackWithTextChildren	mount	4544	4547	5000
SwatchColorPicker	mount	10320	10160	5000
Tabs	mount	1437	1421	1000
TagPicker	mount	2611	2582	5000
TeachingBubble	mount	12115	11993	5000
Text	mount	421	415	5000
TextField	mount	1411	1362	5000
ThemeProvider	mount	1198	1205	5000
ThemeProvider	virtual-rerender	597	618	5000
Toggle	mount	820	826	5000
buttonNative	mount	122	120	5000

Perf Analysis (`@fluentui/react-northstar`)

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
ImageMinimalPerf.default	390	359	1.09:1
VideoMinimalPerf.default	629	579	1.09:1
TreeWith60ListItems.default	175	162	1.08:1
MenuButtonMinimalPerf.default	1709	1618	1.06:1
SkeletonMinimalPerf.default	361	342	1.06:1
PortalMinimalPerf.default	188	179	1.05:1
RefMinimalPerf.default	241	231	1.04:1
TextMinimalPerf.default	352	340	1.04:1
AnimationMinimalPerf.default	405	393	1.03:1
ChatMinimalPerf.default	660	638	1.03:1
CheckboxMinimalPerf.default	2793	2719	1.03:1
ListCommonPerf.default	619	603	1.03:1
ListMinimalPerf.default	523	509	1.03:1
PopupMinimalPerf.default	601	584	1.03:1
ProviderMergeThemesPerf.default	1702	1650	1.03:1
AttachmentMinimalPerf.default	150	147	1.02:1
ButtonSlotsPerf.default	559	549	1.02:1
CarouselMinimalPerf.default	463	455	1.02:1
GridMinimalPerf.default	334	326	1.02:1
ItemLayoutMinimalPerf.default	1232	1211	1.02:1
LabelMinimalPerf.default	391	384	1.02:1
ProviderMinimalPerf.default	984	969	1.02:1
StatusMinimalPerf.default	680	666	1.02:1
TableMinimalPerf.default	417	410	1.02:1
AccordionMinimalPerf.default	148	147	1.01:1
ButtonOverridesMissPerf.default	1677	1666	1.01:1
ChatDuplicateMessagesPerf.default	294	290	1.01:1
DividerMinimalPerf.default	359	356	1.01:1
EmbedMinimalPerf.default	4115	4090	1.01:1
HeaderMinimalPerf.default	363	358	1.01:1
InputMinimalPerf.default	1283	1269	1.01:1
LayoutMinimalPerf.default	364	361	1.01:1
LoaderMinimalPerf.default	719	713	1.01:1
SliderMinimalPerf.default	1584	1569	1.01:1
IconMinimalPerf.default	615	607	1.01:1
TreeMinimalPerf.default	813	804	1.01:1
AlertMinimalPerf.default	282	281	1:1
DropdownManyItemsPerf.default	676	679	1:1
DropdownMinimalPerf.default	3110	3103	1:1
HeaderSlotsPerf.default	757	757	1:1
ListNestedPerf.default	550	548	1:1
ListWith60ListItems.default	633	634	1:1
MenuMinimalPerf.default	842	838	1:1
TableManyItemsPerf.default	1920	1911	1:1
TextAreaMinimalPerf.default	486	485	1:1
CustomToolbarPrototype.default	3821	3835	1:1
ToolbarMinimalPerf.default	920	920	1:1
TooltipMinimalPerf.default	1012	1012	1:1
BoxMinimalPerf.default	347	352	0.99:1
ButtonMinimalPerf.default	169	170	0.99:1
DialogMinimalPerf.default	748	753	0.99:1
FlexMinimalPerf.default	280	284	0.99:1
AvatarMinimalPerf.default	191	195	0.98:1
CardMinimalPerf.default	535	544	0.98:1
DatepickerMinimalPerf.default	5346	5452	0.98:1
FormMinimalPerf.default	397	407	0.98:1
RadioGroupMinimalPerf.default	445	453	0.98:1
ReactionMinimalPerf.default	361	370	0.98:1
SegmentMinimalPerf.default	333	340	0.98:1
SplitButtonMinimalPerf.default	3775	3847	0.98:1
AttachmentSlotsPerf.default	1039	1069	0.97:1
RosterPerf.default	1098	1151	0.95:1
ChatWithPopoverPerf.default	342	374	0.91:1

ling1726 · 2021-07-30T13:29:07Z

+                <EventListener
+                  listener={handleDocumentClick(getRefs)}
+                  target={context.target}
+                  type="mousedown"


it's an interesting solution, you might have to modify tests since, generally tests will only trigger the specific event.

Perhaps this should also be brought up in open hours as an FYI.

…ix/popup-click-outside

…i into fix/popup-click-outside

…icrosoft#19201) * fix(Popup): prevent closing popup on finishing select text outside * fix: tests * fix: test * fix popup * update test * fix test Co-authored-by: Charles Assuncao <charlesassuncao@192.168.1.39>

fix(Popup): prevent closing popup on finishing select text outside

ceb18db

chpalac requested review from jurokapsiar, layershifter, ling1726, miroslavstastny and pompomon as code owners July 30, 2021 11:49

msft-fluent-ui-bot added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label Jul 30, 2021

msft-fluent-ui-bot assigned ling1726 Jul 30, 2021

chpalac changed the title ~~fix(Popup): prevent closing popup on finishing select text outside~~ fix(Popup): prevent closing popup when finishing select text outside Jul 30, 2021

ling1726 reviewed Jul 30, 2021

View reviewed changes

Charles Assuncao and others added 4 commits August 1, 2021 16:20

Merge branch 'master' of https://github.com/microsoft/fluentui into f…

6155da0

…ix/popup-click-outside

fix: tests

c9bf253

Merge branch 'fix/popup-click-outside' of github.com:chassunc/fluentu…

e25e330

…i into fix/popup-click-outside

fix: test

27a8d93

layershifter reviewed Aug 2, 2021

View reviewed changes

Comment thread packages/fluentui/react-northstar/src/components/Popup/Popup.tsx Outdated

chpalac added 3 commits August 2, 2021 08:23

fix popup

8fe2cec

update test

75620f8

fix test

83a7995

layershifter approved these changes Aug 3, 2021

View reviewed changes

chpalac merged commit 31f949a into microsoft:master Aug 3, 2021

YuanboXue-Amber mentioned this pull request Aug 8, 2023

fix(react-utilities): useOnClickOutside should consider text selection from inside to outside as inside click #28765

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(Popup): prevent closing popup when finishing select text outside#19201

fix(Popup): prevent closing popup when finishing select text outside#19201
chpalac merged 8 commits into
microsoft:masterfrom
chpalac:fix/popup-click-outside

chpalac commented Jul 30, 2021

Uh oh!

DustyTheBot commented Jul 30, 2021 •

edited

Loading

Uh oh!

codesandbox-ci Bot commented Jul 30, 2021 •

edited

Loading

Uh oh!

size-auditor Bot commented Jul 30, 2021 •

edited

Loading

Uh oh!

fabricteam commented Jul 30, 2021 •

edited

Loading

Uh oh!

fabricteam commented Jul 30, 2021 •

edited

Loading

Uh oh!

ling1726 Jul 30, 2021

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

chpalac commented Jul 30, 2021

Pull request checklist

Description of changes

Focus areas to test

Uh oh!

DustyTheBot commented Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codesandbox-ci Bot commented Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

size-auditor Bot commented Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

fabricteam commented Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

fabricteam commented Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react)

Perf Analysis (@fluentui/react-northstar)

Uh oh!

ling1726 Jul 30, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DustyTheBot commented Jul 30, 2021 •

edited

Loading

codesandbox-ci Bot commented Jul 30, 2021 •

edited

Loading

size-auditor Bot commented Jul 30, 2021 •

edited

Loading

fabricteam commented Jul 30, 2021 •

edited

Loading

fabricteam commented Jul 30, 2021 •

edited

Loading

Perf Analysis (`@fluentui/react`)

Perf Analysis (`@fluentui/react-northstar`)