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

fix(Popup): fix styling with pointer when it is wrapped by FocusZones #1492

Merged
layershifter merged 3 commits into from
Jun 14, 2019
Merged

fix(Popup): fix styling with pointer when it is wrapped by FocusZones #1492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7117c60
fix(Popup): apply styles `pointer` when it is wrapped with FZ
layershifter Jun 12, 2019
96dee2a
address review comments
layershifter Jun 14, 2019
8e9963e
Merge branch 'master' into fix/popup-pointer-fz
layershifter Jun 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Fixes
- Fix prop types of `Tooltip` component @kuzhelov ([#1499](https://github.com/stardust-ui/react/pull/1499))
- Fix `Popup` styling with `pointer` when it is wrapped by `FocusZones` @layershifter ([#1492](https://github.com/stardust-ui/react/pull/1492))

<!--------------------------------[ v0.33.0 ]------------------------------- -->
## [v0.33.0](https://github.com/stardust-ui/react/tree/v0.33.0) (2019-06-13)
Expand Down
1 change: 1 addition & 0 deletions packages/react/src/components/Popup/Popup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ export default class Popup extends AutoControlledComponent<PopupProps, PopupStat
placement,
pointing,
pointerRef: this.pointerTargetRef,
unstable_wrapped: accessibility.focusTrap || accessibility.autoFocus,
},
overrideProps: this.getContentProps,
})
Expand Down
8 changes: 8 additions & 0 deletions packages/react/src/components/Popup/PopupContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ export interface PopupContentProps

/** A ref to a pointer element. */
pointerRef?: React.Ref<Element>

/**
* @deprecated
* Indicates that PopupContent is wrapped with FocusZone. Do not use it, it used only for internal implementation and
Copy link
Contributor

@kuzhelov kuzhelov Jun 14, 2019

Choose a reason for hiding this comment

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

nit: lets use @deprecated pragma to make proper accent on that - as this pragma is commonly supported and visualised (at least) by IDEs

Copy link
Member Author

@layershifter layershifter Jun 14, 2019

Choose a reason for hiding this comment

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

Will do, but you accidentally tagged someone 😸

* will be removed in future releases.
*/
unstable_wrapped?: boolean
}

class PopupContent extends UIComponent<WithAsProp<PopupContentProps>> {
Expand All @@ -68,6 +75,7 @@ class PopupContent extends UIComponent<WithAsProp<PopupContentProps>> {
onMouseEnter: PropTypes.func,
onMouseLeave: PropTypes.func,
pointerRef: customPropTypes.ref,
unstable_wrapped: PropTypes.bool,
}

static defaultProps = {
Expand Down
4 changes: 4 additions & 0 deletions packages/react/src/themes/teams/components/Popup/popupContentStyles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ const popupContentStyles: ComponentSlotStylesInput<PopupContentProps, PopupConte
borderRadius: v.borderRadius,
display: 'block',

...(p.unstable_wrapped && {
backgroundColor: 'inherit',
Copy link
Collaborator

@bmdalex bmdalex Jun 14, 2019

Choose a reason for hiding this comment

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

Wouldn't it be enough to add the styles all the time? without the condition of p.unstable_wrapped? I mean maybe we can avoid this prop altogether... Or will adding these styles cause different issue?

Copy link
Member Author

@layershifter layershifter Jun 14, 2019

Choose a reason for hiding this comment

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

If we will always add these styles we will inherit backgroundColor from body when there are no wrapper...

position: 'relative',
}),
...(p.pointing && getPointerStyles(v.pointerOffset, v.pointerMargin, rtl, p.placement).root),
}),
pointer: ({ props: p, variables: v, rtl }): ICSSInJSStyle => ({
Expand Down