Coachmark: Positioning fixes by leddie24 · Pull Request #4995 · microsoft/fluentui

leddie24 · 2018-05-25T18:25:56Z

Pull request checklist

Addresses an existing issue: Fixes Teaching Bubble close icon on hover #4812
Include a change request file using $ npm run change

Description of changes

Fixed positioning bugs (Coachmark/TeachingBubble will re-position on scroll/resize properly)
Added in missing positions for Coachmark as well as alignment edges for the TeachingBubble/Beak
Adds in missing beak directions for Beak
Removed configuration props for Coachmark and set them as CONSTs instead
Updated basic example to provide configuration of Coachmark position via Dropdown
Fix TeachingBubble styling for buttons

Coachmark right position:

Coachmark top position:

Coachmark left position

Teaching bubble close button hover fix

Before:

After

Focus areas to test

(optional)

…et element. Fix documentation page to not hide overflow on Coachmark example.

…c-react into coachmarkFixes

….ts. Add string to 'target' type, Add COACHMARK_ATTRIBUTE_NAME. Fix TeachingBubble closeButton selector.

…c-react into coachmarkFixes

…i-fabric-react into coachmarkFixes

…c-react into coachmarkFixes

…erProps. TODO fix Beak transform for different directions. Change Coachmark size and beak size.

…omponent on resize

…sions and set as exported const. Fix scss selectors for TeachingBubble buttons.

…color from Beak props

…chmark is expanded. Fix finalheight prop.

…n't contain element. TODO fix bounds for target rect

…beakDirection in Coachmark

…mark.

…oachmark Do's and Overview.

…emove extra div in PositioningContainer

…c-react into coachmarkFixes

…c-react into coachmarkFixes # Conflicts: # packages/office-ui-fabric-react/src/components/Coachmark/PositioningContainer/PositioningContainer.types.ts

joschect · 2018-05-30T17:38:43Z

...ffice-ui-fabric-react/src/components/Coachmark/PositioningContainer/PositioningContainer.tsx

    );
  }

-  public dismiss = (ev?: Event | React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>): void => {


Since dismiss is public you'll need to keep it around and deprecate it.

joschect · 2018-05-30T17:43:42Z

packages/office-ui-fabric-react/src/components/Coachmark/Coachmark.tsx

+        } else {
+          // Beak is aligned to the top of target
+          if (targetAlignment === RectangleEdge.top) {
+            beakTop = `calc(${COACHMARK_WIDTH / 2}px - ${BEAK_WIDTH / 2}px)`;


Why are you using calc? That's likely going to be slower than just doing the calculation right here in javascript.

joschect · 2018-05-30T17:47:40Z

packages/office-ui-fabric-react/src/components/Coachmark/Coachmark.tsx

    };
  }

+  private get _beakDirection(): BeakDirection {


I'd recommend using rectangleEdge from positioning. Then implement a public "getOppositeEdge" function in positioning which would look like

export function getOppositeEdge(edge: RectangleEdge): RectangleEdge { return edge * -1; }

joschect · 2018-05-30T17:50:15Z

packages/office-ui-fabric-react/src/components/Coachmark/Beak/Beak.types.ts


 export interface IBeak { }

+export enum BeakDirection {


Reuse RectangleEdge from positioning

…g. Remove unnecessary css calc for beak in Coachmark.

micahgodbolt · 2018-05-30T23:22:32Z

@joschect i'm happy to sign off on this a codeowner as long as you are

leddie24 · 2018-05-31T19:23:40Z

Looks like there's some bug only reproing in Windows. The Positioning container cuts off the teaching bubble and doesn't fully extend to its full height.

micahgodbolt · 2018-05-31T20:06:06Z

give me a shout when you've got those bugs wrapped up and i'll approve

…beak.

leddie24 · 2018-06-01T18:55:48Z

@micahgodbolt could you take another look when you get a chance? Thanks!

micahgodbolt · 2018-06-01T18:59:15Z

As long as it all passes, i'm good. @joschect already signed off

* master: (274 commits) Demo Page Refactor Part 1 (microsoft#5055) SplitButton: Add aria-roledescription (microsoft#5062) Add addins sketch toolkit link (microsoft#5052) Dropdown title (microsoft#4983) Allow for more control over event handling for keytips (microsoft#5064) Build time speed improvements (microsoft#4965) Coachmark: Positioning fixes (microsoft#4995) Applying package updates. Experiments/Nav component: display "show more" link only if there is atleast one hidden link (microsoft#5057) Add pointerup listener to exit keytip mode (microsoft#5051) Update PULL_REQUEST_TEMPLATE.md Update ISSUE_TEMPLATE.md Shimmer: resolve conflicts (microsoft#5034) Invalid ARIA attributes: Fix empty values (microsoft#5040) ComboBox: Correct invalid ARIA attributes. (microsoft#4873) (microsoft#5001) ComboBox: Fix submit pending value (microsoft#5048) FocusTrapZone - restore last focused descendant element (microsoft#4897) Applying package updates. Take 2 of the require.resolve change. This time using the "resolve" pkg (microsoft#5031) fixing webpack config to allow rush build to complete on a small VM (microsoft#5037) ...

* master: (111 commits) Moving a variable to be defined sooner so that it is there when failures occur. Fix icon imports (microsoft#5069) MessageBar: More visible HC color for dismiss and expand buttons (microsoft#5061) Fix DetailsList accessibility and add more ARIA hooks (microsoft#5066) Update jest (microsoft#5068) Demo Page Refactor Part 1 (microsoft#5055) SplitButton: Add aria-roledescription (microsoft#5062) Add addins sketch toolkit link (microsoft#5052) Dropdown title (microsoft#4983) Allow for more control over event handling for keytips (microsoft#5064) Build time speed improvements (microsoft#4965) Coachmark: Positioning fixes (microsoft#4995) Applying package updates. Experiments/Nav component: display "show more" link only if there is atleast one hidden link (microsoft#5057) Add pointerup listener to exit keytip mode (microsoft#5051) Update PULL_REQUEST_TEMPLATE.md Update ISSUE_TEMPLATE.md Shimmer: resolve conflicts (microsoft#5034) Invalid ARIA attributes: Fix empty values (microsoft#5040) ComboBox: Correct invalid ARIA attributes. (microsoft#4873) (microsoft#5001) ...

…5656) * Cherry pick Coachmark: Positioning fixes (#4995) e87289b * Cherry pick TeachingBubble/Coachmark: Fix TeachingBubble content wrapping to next line unnecessarily (#5121) 72bab3c * Cherrry pick Fix import paths to use relative paths in product code (#5338) e2076e1 * Cherry pick Coachmark accessibility #5155 7a36bb9 * Revert formatting back to 5.0 in Coachmark.tsx * Fix unnecessary ; and change " to ' * Fix merge conflicts for Coachmark.Basic.Example.tsx * " to ' * Fix imports for TeachingBubble.tsx * Update TeachinBubbleContent, update TeachingBubble snapshot * Update snapshot. Fix TeachingBubble.types * Fix import for RefObject * Fix some inadvertent formatting from prettier.

edwlmsft added 30 commits April 23, 2018 21:21

Change Coachmark to doNotLayer={false} so component scrolls with targ…

de04c2b

…et element. Fix documentation page to not hide overflow on Coachmark example.

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

7fa91be

…c-react into coachmarkFixes

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

8392717

…c-react into coachmarkFixes

Test changes

d38c04f

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

bf9032e

…c-react into coachmarkFixes

TODO add parameter for setting rectangle out of bounds in positioning…

97b3d82

….ts. Add string to 'target' type, Add COACHMARK_ATTRIBUTE_NAME. Fix TeachingBubble closeButton selector.

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

74580e4

…c-react into coachmarkFixes

Merge branch 'coachmarkFixes' of https://github.com/leddie24/office-u…

14aefc3

…i-fabric-react into coachmarkFixes

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

c1e9671

…c-react into coachmarkFixes

Work in progress. Adding Beak direction and adding positioningContain…

d5de226

…erProps. TODO fix Beak transform for different directions. Change Coachmark size and beak size.

export BEAK_HEIGHT/WIDTH, add smart positioning to beak. Reposition c…

f7713f5

…omponent on resize

Fix coachmark positioning for collapsed/open states

9b94977

Remove props for width/height for coachmark dimensions and beak dimen…

1663d7d

…sions and set as exported const. Fix scss selectors for TeachingBubble buttons.

Fix linting errors. Reset positioning back to origin master.

a44d082

Add allowOutOfBounds parameter for positioning

45b4f1a

Set mouse proximity default to 10. Fix mouse proximity check. Remove …

20032e9

…color from Beak props

Change collapsed --> isCollasped. Fix onmousemove triggering when coa…

630606c

…chmark is expanded. Fix finalheight prop.

Fix lint

e380113

Fix finalHeight set state bug. Set positions to undefined if DOM does…

f1f370c

…n't contain element. TODO fix bounds for target rect

Move allowOutsideOfBounds to PositioningContainer. Add comments for _…

17e9355

…beakDirection in Coachmark

Set _targetElementRect in Coachmark. Fix isMouseInProximity

804a49f

Only update components if state/prop changes. Fix onPosition in Coach…

bc54099

…mark.

Fix TeachingBubble footer

257f8fb

Remove allowOutOfBounds prop. Add in bounds logic to Coachmark. Fix C…

dac3f0f

…oachmark Do's and Overview.

Reduce throttle frequency

0ac7ef4

Fix animation end event. Change opacity to 1.

ecf2a06

Fix _openCoachmark animation open

35b7d0b

Deprecate props. Fix specificity for TeachingBubble.

8e18abf

Remove isScrollable prop on CoachmarkPage. Fix import order in Beak R…

cf7e03c

…emove extra div in PositioningContainer

Tidy up imports in Coachmark

959cb80

leddie24 requested review from cschlechty and joschect May 25, 2018 18:25

edwlmsft added 4 commits May 25, 2018 11:26

Remove unneeded css rule

2c04bce

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

ec399b0

…c-react into coachmarkFixes

Add dropdownContainer to ICoachmarkBasicExampleStyles

d475f50

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

c952a2c

…c-react into coachmarkFixes # Conflicts: # packages/office-ui-fabric-react/src/components/Coachmark/PositioningContainer/PositioningContainer.types.ts

joschect reviewed May 30, 2018

View reviewed changes

edwlmsft added 3 commits May 30, 2018 13:40

Remove BeakDirection enum. Add getOppositeEdge function to positionin…

b8ac188

…g. Remove unnecessary css calc for beak in Coachmark.

Deprecate dismiss

1b06b3f

Add change file

f15504c

Update Do's

c72ddcd

joschect approved these changes May 31, 2018

View reviewed changes

edwlmsft added 2 commits June 1, 2018 11:35

Add comments to props. Fix auto edge positioning. Fix color prop for …

4ff1c48

…beak.

Add more comments

3e21c86

micahgodbolt approved these changes Jun 1, 2018

View reviewed changes

edwlmsft added 3 commits June 1, 2018 12:04

Remove color prop in example

3dd8303

Undeprecate color prop in Beak

d702ed8

Comment change

c8222a3

leddie24 merged commit e87289b into microsoft:master Jun 1, 2018

leddie24 deleted the coachmarkFixes branch June 1, 2018 20:24

leddie24 mentioned this pull request Jul 23, 2018

Cherry pick Coachmark accessibility and positioning fixes into 5.0 #5656

Merged

2 tasks

microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Coachmark: Positioning fixes#4995

Coachmark: Positioning fixes#4995
leddie24 merged 51 commits intomicrosoft:masterfrom
leddie24:coachmarkFixes

leddie24 commented May 25, 2018 •

edited

Loading

Uh oh!

joschect May 30, 2018

Uh oh!

joschect May 30, 2018

Uh oh!

joschect May 30, 2018

Uh oh!

joschect May 30, 2018

Uh oh!

micahgodbolt commented May 30, 2018

Uh oh!

leddie24 commented May 31, 2018

Uh oh!

micahgodbolt commented May 31, 2018

Uh oh!

leddie24 commented Jun 1, 2018

Uh oh!

micahgodbolt commented Jun 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

leddie24 commented May 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Description of changes

Focus areas to test

Uh oh!

joschect May 30, 2018

Choose a reason for hiding this comment

Uh oh!

joschect May 30, 2018

Choose a reason for hiding this comment

Uh oh!

joschect May 30, 2018

Choose a reason for hiding this comment

Uh oh!

joschect May 30, 2018

Choose a reason for hiding this comment

Uh oh!

micahgodbolt commented May 30, 2018

Uh oh!

leddie24 commented May 31, 2018

Uh oh!

micahgodbolt commented May 31, 2018

Uh oh!

leddie24 commented Jun 1, 2018

Uh oh!

micahgodbolt commented Jun 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

leddie24 commented May 25, 2018 •

edited

Loading