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

fix(Popup): fix broken background with pointing and FocusTrap #1689

Merged
merged 5 commits into from
Jul 19, 2019

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Jul 19, 2019

Fixes #1679.

Problem

In master we have already resolved this issue by #1565, however we need to provide a fix without breaking changes for now.

Before

image

After

image

Test snippet

import * as React from 'react'
import { Button, Flex, Popup, popupFocusTrapBehavior } from '@stardust-ui/react'

const plainContentStyle = {
  zIndex: 1000,
  padding: 5,
}

const PopupExample = () => (
  <Flex vAlign='center' space='between' styles={{ padding: '50px', height: '500px', backgroundColor: 'orange' }}>
    <Popup content="Hello from popup!" pointing open accessibility={popupFocusTrapBehavior}>
      <Button icon="expand"/>
    </Popup>

    <Popup open content={<p style={plainContentStyle}>Plain popup content rendered 'as is'.</p>}>
      <Button icon="expand" content="Popup with plain content"/>
    </Popup>
    <Popup open accessibility={popupFocusTrapBehavior}
           content={<p style={plainContentStyle}>Plain popup content rendered 'as is'.</p>}>
      <Button icon="expand" content="Popup with plain content"/>
    </Popup>

    <Popup
      open
      content={{ content: <p style={plainContentStyle}>Popup content rendered in wrapper.</p> }}
    >
      <Button icon="expand" content="Popup with wrapped content"/>
    </Popup>
  </Flex>
)

export default PopupExample

@DustyTheBot
Copy link
Collaborator

DustyTheBot commented Jul 19, 2019

Fails
🚫 All of your entries in CHANGELOG.md should be in the **Unreleased** section!

Generated by 🚫 dangerJS

@codecov
Copy link

codecov bot commented Jul 19, 2019

Codecov Report

Merging #1689 into chore/0.34.2 will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@               Coverage Diff                @@
##           chore/0.34.2    #1689      +/-   ##
================================================
- Coverage         71.43%   71.42%   -0.02%     
================================================
  Files               848      848              
  Lines              6942     6943       +1     
  Branches           2007     1988      -19     
================================================
  Hits               4959     4959              
- Misses             1977     1978       +1     
  Partials              6        6
Impacted Files Coverage Δ
...t/src/themes/teams/components/Popup/popupStyles.ts 25% <0%> (-8.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 379d928...6d9b6a4. Read the comment docs.

@layershifter layershifter marked this pull request as ready for review July 19, 2019 13:27
@layershifter layershifter changed the base branch from master to chore/0.34.2 July 19, 2019 14:51
/*
* This fix handles all cases with wrapped focus trap.
*/
...(React.isValidElement(p.content) && {
Copy link
Contributor

Choose a reason for hiding this comment

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

actually, this should also apply in case if render function is provided as an argument :( as in that case slot component will be also replaced. So, I think that we may just check that this is not an object or primitive

@layershifter
Copy link
Member Author

Adjusted to handle render() callback cases:

    <Popup open trigger={<Button icon="expand"/>} content={render => render(
      {},
      (ComponentType, props) => (
        <p style={{ zIndex: 1000, padding: 5 }}>Plain popup content rendered 'as is'.</p>
      ),
    )}/>
    <Popup open trigger={<Button icon="expand"/>} pointing content={render => render(
      {},
      (ComponentType, props) => (
        <div {...props} style={{ zIndex: 1000, padding: 5}}>
          Plain popup content rendered 'as is'.
        </div>
      ),
    )}/>

image

@layershifter layershifter merged commit a805341 into chore/0.34.2 Jul 19, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/popup-pointer branch July 19, 2019 15:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants