Refactor PostVisibility to use Popover component

[aduth]

Related: #1193

This pull request seeks to extend generalized Popover behavior from #1193, refactoring PostVisibility to make use of the common component in place of its specific implementation. Refactoring will continue in subsequent pull requests to also revise the PostSchedule component. The changes here required some additional effort to improve Popover to be less specific to the Inserter use-case.

Implementation notes:

The Popover component now renders content within a nested div. The reason for this is to allow the root node to be 0px by 0px, positioned absolutely at the edge relative to the parent in which it is rendered. This ensures that the arrow is always shown exactly at the anchor point, and content is adjusted per the desired direction.

Design Notes:

Some design changes were necessary to better serve general Popover usage. Previously the left-bound inserter in post content had extended further to the left. Attempting to mirror this in the sidebar visibility dialog proved to exceed the page boundaries, so it was brought tighter in line with the node to which the dialog is anchored. Additionally, the editor header inserter Popover no longer opens anchored to the "+" icon but instead centered to the entire "+ Insert" button text.

Testing instructions:

While this pull request is targeted at PostVisibility, due to refactoring of the Popover component, additional testing should be done on the Inserter (small and large viewports) to ensure that no regressions have been introduced.

Repeat testing instructions from #1193.

Verify that the Post Visibility dialog appears and behaves more-or-less identical as it does in the master branch.

Follow-up Tasks:

• Refactor PostSchedule
• Move onClickOutside behavior to Popover, accepting an onClose prop handler

[aduth]

Majority of changes here are simply to wrap the button and Popover in a span to which a position: relative; style is applied, allowing the Popover to position itself within (necessary since div is not a valid child of button).

Using something like react-portal could allow us to be less particular about the DOM structure or relative positioning. Portals are apparently supported in React 16 out-of-the-box, but I am not sure if they are documented yet.

[aduth]

Rebased to resolve conflicts. This would be a good one to move forward on because we're having to deal with many implementation-specific requirements that would be good to consolidate (e.g. click outside, blur, etc).

[jasmussen]

Could be due to changes recently happening in master, but I'm seeing a little weirdness with both the inserter and the visibility popover:

Edit: Again I had to checkout using origin/update/visibility-popover. Don't know why my git is so broken.

[aduth]

@jasmussen I just rebased the branch, and am unable to reproduce. Could you try deleting your local copy and giving it another go? If you're still having trouble, could you let me know which browser / version you're using?

[codecov-io]

Codecov Report

Merging #1204 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master    #1204   +/-   ##
=======================================
  Coverage   20.34%   20.34%           
=======================================
  Files         135      135           
  Lines        4237     4237           
  Branches      722      722           
=======================================
  Hits          862      862           
  Misses       2843     2843           
  Partials      532      532

Impacted Files	Coverage Δ
editor/sidebar/post-visibility/index.js	0% <0%> (ø)	⬆️
components/popover/index.js	0% <0%> (ø)	⬆️

---

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 e514290...5a58688. Read the comment docs.

[jasmussen]

I now see no diff. between master and this branch. Which sounds like a success criteria, so 👍 👍!

I do see this, though:

But I see that also in master, and it seems to be related to #2059

[aduth]

Currently this should only affect the visibility popover, not the date picker, but it's intended to be part of a series toward consolidating patterns to a common component (similar to DropDownMenu, #1880).

I'll plan to merge this one after today's release and address schedule popover in a subsequent pull request.

[jasmussen]

👍👍

[aduth]

In testing on mobile, I found that the inserter popover was not behaving correctly, but then I noticed the same in master. It appears that our logic for calculating the height of the inserter content was never updated when we added tabs to the inserter. This is accounted for in 5a58688.