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

feat(Popup): make 'content' prop to be a shorthand #322

Merged
merged 8 commits into from
Oct 8, 2018

Conversation

kuzhelov
Copy link
Contributor

@kuzhelov kuzhelov commented Oct 5, 2018

TODO

  • introduce examples for wrapped/non-wrapped Popup.
  • update changelog

These BREAKING changes make content to be a shorthand prop of the Popup, to address #312. With that the API will take the following look.

Unwrapped popup content, pasted 'as is'

const popupStyle = {
  zIndex: 1000,
 ...
}

<Popup content={<p style={popupStyle}>Hello from popup!</p>}>} ... >
   ...
</Popup>
<html>
  ...
  <body>
    ...
    <p>...</p>  <----- popup content element, pasted 'as is'
  </body>
</html>

Wrapped one (with PopupContent)

<Popup content='Hello from wrapped popup content!' ... >
   ...
</Popup>
<html>
  ...
  <body>
    ...
    <div class="ui-popup__content">  <----- popup wrapper
      Hello from wrapped popup!   <----- popup content
    </div>
  </body>
</html>

Wrapped one - with custom wrapped content

<Popup content={{ content: <Input ... /> }} ... >
   ...
</Popup>
<html>
  ...
  <body>
    ...
    <div class="ui-popup__content">  <----- popup wrapper
      <div class="ui-input">    <----- popup content
        ...
      </div>
    </div>
  </body>
</html>

@codecov
Copy link

codecov bot commented Oct 5, 2018

Codecov Report

Merging #322 into master will increase coverage by 0.03%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #322      +/-   ##
==========================================
+ Coverage   89.55%   89.59%   +0.03%     
==========================================
  Files          62       62              
  Lines        1216     1220       +4     
  Branches      155      179      +24     
==========================================
+ Hits         1089     1093       +4     
  Misses        125      125              
  Partials        2        2
Impacted Files Coverage Δ
src/components/Popup/PopupContent.tsx 100% <100%> (ø) ⬆️
src/components/Popup/Popup.tsx 41.37% <50%> (ø) ⬆️

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 6878534...757242f. Read the comment docs.

Copy link
Contributor

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

I like this approach, as it fixes the styling issues for the content of the Popup.

@kuzhelov kuzhelov merged commit 763973a into master Oct 8, 2018
@layershifter layershifter deleted the feat/popup-content-shorthand branch November 9, 2018 09:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants