Skip to content

Commit 6259f5d

Browse files
JasonVMoJasonVMo
authored
Components: Callout, fix positioning issues on windows and macOSUser/jasonvmo/callout issue 4291 (#4292)
* fix callout positioning on macos * update authoring instructions for native components and move Callout to a native section in the storybook * fix callout issues on windows * add inadvertant add of win32 storybook tests to e2e pipeline
1 parent 1998ebd commit 6259f5d

9 files changed

Lines changed: 108 additions & 36 deletions

File tree

‎.changeset/calm-callouts-align.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@fluentui-react-native/callout": patch
3+
---
4+
5+
Fix macOS placement directions and anchor Windows Callouts to the requested target edge.

‎.github/skills/agentic-component-authoring/references/tests-and-stories.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,9 @@ resizes or re-roles the component instead of demonstrating a value.
9494

9595
Each story module should provide:
9696

97-
- typed `Meta` with `component` and `Components/<Name>` or `Primitives/<Name>` title
97+
- typed `Meta` with `component` and a title matching the component location:
98+
`Components/<Name>` for higher-order components, `Primitives/<Name>` for agentic primitives, or `Native/<Name>` for
99+
standalone native component packages
98100
- useful common args
99101
- controls for finite or numeric public props
100102
- a short component description

‎.github/workflows/pr.yml‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -406,10 +406,6 @@ jobs:
406406
run: yarn e2etest:win32
407407
working-directory: apps/E2E
408408

409-
- name: Run Storybook Win32 smoke tests
410-
run: yarn win32:ci
411-
working-directory: apps/storybook
412-
413409
- name: Upload E2E Win32 artifacts
414410
if: ${{ always() }}
415411
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1

‎apps/storybook/scripts/smoke-stories.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"artifactName": "icon-default"
1111
},
1212
{
13-
"storyId": "primitives-callout--default",
13+
"storyId": "native-callout--default",
1414
"testId": "agentic-storybook-callout-trigger",
1515
"statusTestId": "agentic-storybook-callout-status",
1616
"artifactName": "callout-default"

‎apps/storybook/windows-tests/storybook-smoke.test.cjs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,46 @@ test('moves focus between Button Overview controls after a click', async () => {
4848
expect(await secondary.getAttribute('HasKeyboardFocus')).toBe('True');
4949
});
5050

51+
const calloutPlacements = [
52+
{
53+
hint: 'topCenter',
54+
isCorrect: (trigger, callout) => callout.y + callout.height <= trigger.y,
55+
},
56+
{
57+
hint: 'rightCenter',
58+
isCorrect: (trigger, callout) => callout.x >= trigger.x + trigger.width,
59+
},
60+
{
61+
hint: 'bottomCenter',
62+
isCorrect: (trigger, callout) => callout.y >= trigger.y + trigger.height,
63+
},
64+
{
65+
hint: 'leftCenter',
66+
isCorrect: (trigger, callout) => callout.x + callout.width <= trigger.x,
67+
},
68+
];
69+
70+
test.each(calloutPlacements)('anchors the Callout in the $hint direction', async ({ hint, isCorrect }) => {
71+
await selectStory('components-button--default');
72+
await selectStory('native-callout--placement');
73+
74+
const trigger = await app.findElementByTestID(`agentic-storybook-callout-placement-${hint}-trigger`);
75+
await trigger.waitForDisplayed({ timeout: 30000 });
76+
await trigger.click();
77+
78+
const callout = await app.findElementByTestID('agentic-storybook-callout-placement-content');
79+
await callout.waitForDisplayed({ timeout: 30000 });
80+
81+
const [triggerLocation, triggerSize, calloutLocation, calloutSize] = await Promise.all([
82+
trigger.getLocation(),
83+
trigger.getSize(),
84+
callout.getLocation(),
85+
callout.getSize(),
86+
]);
87+
88+
expect(isCorrect({ ...triggerLocation, ...triggerSize }, { ...calloutLocation, ...calloutSize })).toBe(true);
89+
});
90+
5191
test.each([
5292
['components-tag--default', 'agentic-storybook-tag'],
5393
['components-accordion--default', 'accordion-header'],

‎packages/native/Callout/macos/FRNCalloutManager.m‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,23 @@ + (NSRect)screenRect:(id)json
1616
return NSMakeRect(x, y, width, height);
1717
}
1818

19-
// Collapse the directional hint options to the 4 NSRectEdge options
19+
// Collapse the directional hint options to the 4 NSRectEdge options.
20+
// CalloutView uses MinY for placement above the anchor and MaxY for placement below it.
2021
RCT_ENUM_CONVERTER(NSRectEdge, (@{
2122
@"leftTopEdge": @(NSRectEdgeMinX),
2223
@"leftCenter": @(NSRectEdgeMinX),
2324
@"leftBottomEdge": @(NSRectEdgeMinX),
24-
@"topLeftEdge": @(NSRectEdgeMaxY),
25-
@"topAutoEdge": @(NSRectEdgeMaxY),
26-
@"topCenter": @(NSRectEdgeMaxY),
27-
@"topRightEdge": @(NSRectEdgeMaxY),
25+
@"topLeftEdge": @(NSRectEdgeMinY),
26+
@"topAutoEdge": @(NSRectEdgeMinY),
27+
@"topCenter": @(NSRectEdgeMinY),
28+
@"topRightEdge": @(NSRectEdgeMinY),
2829
@"rightTopEdge": @(NSRectEdgeMaxX),
2930
@"rightCenter": @(NSRectEdgeMaxX),
3031
@"rightBottomEdge": @(NSRectEdgeMaxX),
31-
@"bottomLeftEdge": @(NSRectEdgeMinY),
32-
@"bottomAutoEdge": @(NSRectEdgeMinY),
33-
@"bottomCenter": @(NSRectEdgeMinY),
34-
@"bottomRightEdge": @(NSRectEdgeMinY),
32+
@"bottomLeftEdge": @(NSRectEdgeMaxY),
33+
@"bottomAutoEdge": @(NSRectEdgeMaxY),
34+
@"bottomCenter": @(NSRectEdgeMaxY),
35+
@"bottomRightEdge": @(NSRectEdgeMaxY),
3536
}), NSRectEdgeMaxY, integerValue);
3637

3738
@end

‎packages/native/Callout/macos/RCTCalloutComponentView.mm‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
static NSRectEdge RCTNSRectEdgeFromDirectionalHint(CalloutDirectionalHint hint)
2121
{
22+
// CalloutView uses MinY for placement above the anchor and MaxY for placement below it.
2223
switch (hint) {
2324
case CalloutDirectionalHint::LeftTopEdge:
2425
case CalloutDirectionalHint::LeftCenter:
@@ -28,7 +29,7 @@ static NSRectEdge RCTNSRectEdgeFromDirectionalHint(CalloutDirectionalHint hint)
2829
case CalloutDirectionalHint::TopAutoEdge:
2930
case CalloutDirectionalHint::TopCenter:
3031
case CalloutDirectionalHint::TopRightEdge:
31-
return NSRectEdgeMaxY;
32+
return NSRectEdgeMinY;
3233
case CalloutDirectionalHint::RightTopEdge:
3334
case CalloutDirectionalHint::RightCenter:
3435
case CalloutDirectionalHint::RightBottomEdge:
@@ -37,7 +38,7 @@ static NSRectEdge RCTNSRectEdgeFromDirectionalHint(CalloutDirectionalHint hint)
3738
case CalloutDirectionalHint::BottomAutoEdge:
3839
case CalloutDirectionalHint::BottomCenter:
3940
case CalloutDirectionalHint::BottomRightEdge:
40-
return NSRectEdgeMinY;
41+
return NSRectEdgeMaxY;
4142
}
4243
}
4344

‎packages/native/Callout/src/Callout.stories.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ const PlacementExample = (props: CalloutProps) => {
116116
onPress={() => setSelectedIndex(index)}
117117
ref={targetRefs[index]}
118118
style={({ pressed }) => [styles.trigger, pressed && styles.triggerPressed]}
119+
testID={`agentic-storybook-callout-placement-${hint}-trigger`}
119120
>
120121
<Text style={styles.triggerText}>{label}</Text>
121122
</Pressable>
@@ -128,7 +129,7 @@ const PlacementExample = (props: CalloutProps) => {
128129
onDismiss={() => setSelectedIndex(undefined)}
129130
target={targetRefs[selectedIndex]}
130131
>
131-
<View style={styles.calloutContent} collapsable={false}>
132+
<View accessible style={styles.calloutContent} collapsable={false} testID="agentic-storybook-callout-placement-content">
132133
<Text style={styles.heading}>{placements[selectedIndex].label}</Text>
133134
<Text style={styles.body}>Click outside the native window to dismiss it.</Text>
134135
</View>
@@ -139,7 +140,7 @@ const PlacementExample = (props: CalloutProps) => {
139140
};
140141

141142
const meta: Meta<typeof Callout> = {
142-
title: 'Primitives/Callout',
143+
title: 'Native/Callout',
143144
component: Callout,
144145
args: {
145146
accessibilityLabel: 'Callout example',
@@ -213,6 +214,10 @@ const styles = StyleSheet.create({
213214
marginTop: 6,
214215
},
215216
calloutContent: {
217+
backgroundColor: '#ffffff',
218+
borderColor: '#d1d1d1',
219+
borderRadius: 8,
220+
borderWidth: 1,
216221
padding: 16,
217222
width: 280,
218223
},

‎packages/native/Callout/windows/Callout/Callout.cpp‎

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -297,44 +297,66 @@ struct CalloutComponentView
297297
CompositionUIService::ComponentFromReactTag(
298298
m_reactContext.Handle(), Props()->target.AsInt64());
299299
auto targetPos = ViewToScreenOffset(targetView);
300+
auto targetScaleFactor =
301+
targetView.LayoutMetrics().PointScaleFactor;
302+
auto targetWidthPx = static_cast<int32_t>(
303+
targetView.LayoutMetrics().Frame.Width * targetScaleFactor);
304+
auto targetHeightPx = static_cast<int32_t>(
305+
targetView.LayoutMetrics().Frame.Height * targetScaleFactor);
306+
auto targetRight = targetPos.X + targetWidthPx;
307+
auto targetBottom = targetPos.Y + targetHeightPx;
308+
auto targetCenterX = targetPos.X + targetWidthPx / 2;
309+
auto targetCenterY = targetPos.Y + targetHeightPx / 2;
300310

301311
POINT anchorPoint{targetPos.X, targetPos.Y};
302312
SIZE windowSize{clientWidthPx, clientHeightPx};
303313

304-
RECT excludeRect{targetPos.X, targetPos.Y,
305-
targetPos.X + targetView.LayoutMetrics().Frame.Width,
306-
targetPos.Y + targetView.LayoutMetrics().Frame.Height};
314+
RECT excludeRect{targetPos.X, targetPos.Y, targetRight, targetBottom};
307315

308316
UINT flags = 0;
309317

310318
if (m_directionalHint == DirectionalHint::LeftTopEdge) {
311-
flags = TPM_LEFTALIGN | TPM_TOPALIGN | TPM_HORIZONTAL;
319+
anchorPoint = {targetPos.X, targetPos.Y};
320+
flags = TPM_RIGHTALIGN | TPM_TOPALIGN | TPM_HORIZONTAL;
312321
} else if (m_directionalHint == DirectionalHint::LeftCenter) {
313-
flags = TPM_LEFTALIGN | TPM_VCENTERALIGN | TPM_HORIZONTAL;
322+
anchorPoint = {targetPos.X, targetCenterY};
323+
flags = TPM_RIGHTALIGN | TPM_VCENTERALIGN | TPM_HORIZONTAL;
314324
} else if (m_directionalHint == DirectionalHint::LeftBottomEdge) {
315-
flags = TPM_LEFTALIGN | TPM_BOTTOMALIGN | TPM_HORIZONTAL;
325+
anchorPoint = {targetPos.X, targetBottom};
326+
flags = TPM_RIGHTALIGN | TPM_BOTTOMALIGN | TPM_HORIZONTAL;
316327
} else if (m_directionalHint == DirectionalHint::TopLeftEdge) {
317-
flags = TPM_LEFTALIGN | TPM_TOPALIGN | TPM_VERTICAL;
328+
anchorPoint = {targetPos.X, targetPos.Y};
329+
flags = TPM_LEFTALIGN | TPM_BOTTOMALIGN | TPM_VERTICAL;
318330
} else if (m_directionalHint == DirectionalHint::TopAutoEdge) {
319-
flags = TPM_LEFTALIGN | TPM_TOPALIGN | TPM_VERTICAL;
331+
anchorPoint = {targetPos.X, targetPos.Y};
332+
flags = TPM_LEFTALIGN | TPM_BOTTOMALIGN | TPM_VERTICAL;
320333
} else if (m_directionalHint == DirectionalHint::TopCenter) {
321-
flags = TPM_CENTERALIGN | TPM_TOPALIGN | TPM_VERTICAL;
334+
anchorPoint = {targetCenterX, targetPos.Y};
335+
flags = TPM_CENTERALIGN | TPM_BOTTOMALIGN | TPM_VERTICAL;
322336
} else if (m_directionalHint == DirectionalHint::TopRightEdge) {
323-
flags = TPM_RIGHTALIGN | TPM_TOPALIGN | TPM_VERTICAL;
337+
anchorPoint = {targetRight, targetPos.Y};
338+
flags = TPM_RIGHTALIGN | TPM_BOTTOMALIGN | TPM_VERTICAL;
324339
} else if (m_directionalHint == DirectionalHint::RightTopEdge) {
325-
flags = TPM_RIGHTALIGN | TPM_TOPALIGN | TPM_HORIZONTAL;
340+
anchorPoint = {targetRight, targetPos.Y};
341+
flags = TPM_LEFTALIGN | TPM_TOPALIGN | TPM_HORIZONTAL;
326342
} else if (m_directionalHint == DirectionalHint::RightCenter) {
327-
flags = TPM_RIGHTALIGN | TPM_VCENTERALIGN | TPM_HORIZONTAL;
343+
anchorPoint = {targetRight, targetCenterY};
344+
flags = TPM_LEFTALIGN | TPM_VCENTERALIGN | TPM_HORIZONTAL;
328345
} else if (m_directionalHint == DirectionalHint::RightBottomEdge) {
329-
flags = TPM_RIGHTALIGN | TPM_BOTTOMALIGN | TPM_HORIZONTAL;
346+
anchorPoint = {targetRight, targetBottom};
347+
flags = TPM_LEFTALIGN | TPM_BOTTOMALIGN | TPM_HORIZONTAL;
330348
} else if (m_directionalHint == DirectionalHint::BottomLeftEdge) {
331-
flags = TPM_LEFTALIGN | TPM_BOTTOMALIGN | TPM_VERTICAL;
349+
anchorPoint = {targetPos.X, targetBottom};
350+
flags = TPM_LEFTALIGN | TPM_TOPALIGN | TPM_VERTICAL;
332351
} else if (m_directionalHint == DirectionalHint::BottomAutoEdge) {
333-
flags = TPM_LEFTALIGN | TPM_BOTTOMALIGN | TPM_VERTICAL;
352+
anchorPoint = {targetPos.X, targetBottom};
353+
flags = TPM_LEFTALIGN | TPM_TOPALIGN | TPM_VERTICAL;
334354
} else if (m_directionalHint == DirectionalHint::BottomCenter) {
335-
flags = TPM_CENTERALIGN | TPM_BOTTOMALIGN | TPM_VERTICAL;
355+
anchorPoint = {targetCenterX, targetBottom};
356+
flags = TPM_CENTERALIGN | TPM_TOPALIGN | TPM_VERTICAL;
336357
} else if (m_directionalHint == DirectionalHint::BottomRightEdge) {
337-
flags = TPM_RIGHTALIGN | TPM_BOTTOMALIGN | TPM_VERTICAL;
358+
anchorPoint = {targetRight, targetBottom};
359+
flags = TPM_RIGHTALIGN | TPM_TOPALIGN | TPM_VERTICAL;
338360
}
339361

340362
flags |= TPM_WORKAREA;

0 commit comments

Comments
 (0)