Migrating Coachmark to main Package, Added a beak component and updated experiment PositioningContainer.#3919
Conversation
…issue. added a Triangle helper class.
…bric-react into component/beak
…ptions to the coachmark.
|
@cschlechty right now it does only open for mouse but i can add the functionality for opening it on focus or something similar. It shouldn't take much time so i will add that to this pull request. |
| @@ -0,0 +1,41 @@ | |||
| import { IStyle, DefaultPalette } from '../../../Styling'; | |||
|
|
|||
| export interface IBeakStylesProps { | |||
There was a problem hiding this comment.
the interfaces should be in .types.
| return ( | ||
| <div | ||
| className={ css('ms-Beak', classNames.root) } | ||
| ref={ this._resolveRef('_beakElement') } |
There was a problem hiding this comment.
This ref seems unneeded. Please remove it.
| /** | ||
| * A triangle abstraction class used to make calculations | ||
| */ | ||
| export class Triangle { |
…bric-react into component/beak
joschect
left a comment
joschect
left a comment
There was a problem hiding this comment.
Approved with some suggestions
| height: props.height | ||
| }, | ||
| (props.left && props.top) && { | ||
| left: props.left, |
There was a problem hiding this comment.
These cannot be in styles, getClass will automatically flip them to right in RTL which could make it not right.
| } | ||
|
|
||
| public componentDidMount(): void { | ||
| if (this.state.isMeasuring) { |
There was a problem hiding this comment.
componentDidMount is only ever called once per lifecycle and you don't use measuring anywhere else so you can remove this check.
|
|
||
| public componentDidMount(): void { | ||
| if (this.state.isMeasuring) { | ||
| this._async.setTimeout((): void => { |
There was a problem hiding this comment.
use this._async.requestAnimationFrame( ...) instead. It helps ensure that first dom changes occur, then when those are done all the measures can happen.
| }); | ||
| } | ||
|
|
||
| private _isElementInProximity(mouseProximityOffset: number = 0): void { |
There was a problem hiding this comment.
I might rename this something like _addProximityHandler or something, since it should only ever be called once and adds a proximity handler and doesn't actively check for proximity itself.
* master: (71 commits) Applying package updates. Delete initials_2018-02-07-13-49.json Delete initials_2018-02-07-13-49.json Delete jolore-addingWorkWeekDateRange_2018-01-24-01-39.json Delete initials_2018-02-07-13-49.json Delete jolore-addingWorkWeekDateRange_2018-01-24-01-39.json Delete initials_2018-02-07-13-49.json Update .npmrc Cleaning Up Console Log in CommandBar Test (microsoft#4011) Convert Overlay to mergeStyles (microsoft#3978) ScrollablePane SCSS to MergeStyles Part 1: File Structure (microsoft#4008) [ContextualMenu] Fixes useTargetWidth property (microsoft#3943) DetailsList: Consider groups when setting aria-rowcount List: Add a _notifyPageChanges function (microsoft#3990) Applying package updates. Migrating Coachmark to main Package, Added a beak component and updated experiment PositioningContainer. (microsoft#3919) Update package.json Added enum for triggering menu with arrow keys and bool to allow it or not (microsoft#3950) BaseExtendedPicker: Hook up onPaste (microsoft#3885) FocusUtil: fix getPreviousElement to include previous sibling elements correctly (microsoft#3928) ...
4 participants
Pull request checklist
$ npm run changeDescription of changes
Moves the Coachmark over to the main package. Includes several bug fixes, code cleanup and new Beak components
Focus areas to test
Coachmark