add tooltip and popover components #442

jlmitch5 · 2018-07-03T20:18:15Z

fixes #184 #113

patternfly-build · 2018-07-03T20:20:10Z

Deploy preview for pf-next ready!

Built with commit 0daf1d8

https://deploy-preview-442--pf-next.netlify.com

christianvogt · 2018-07-03T20:31:34Z

The header layout with regards to the title and close button should be the same as the modal. cc @mattnolting

jlmitch5 · 2018-07-03T20:47:52Z

good idea @christianvogt I'll add it to my TODOs

dgutride · 2018-07-04T12:37:45Z

src/patternfly/components/Popover/popover.hbs

+      </div>
+    </div>
+  </div>
+  {{!-- {{> popoverParent }} --}}


Finally figured this out over here - popoverParent should be kebab-cased and needs to be defined slightly differently in the base component:

{{#> popover-parent }}
{{/popover-parent}}

this is my entire component from the code we shared before:

<div class="pf-c-tooltip {{pf-c-tooltip-modifier}}"> <div class="pf-c-tooltip__container"> <div class="pf-c-tooltip__arrow"></div> <div class="pf-c-tooltip__content"> {{> @partial-block }} </div> </div> {{#> tooltip-parent }} {{/tooltip-parent}} </div>

dgutride · 2018-07-04T12:39:00Z

src/patternfly/components/Popover/examples/popover-inverted-example.hbs

+    really, really, really, really, really, really, really, really, really, really, really, 
+    really, really, really, really, really, really, really, really, really, really, really, 
+    really, really, long as well as shor
+    {{#*inline "popoverParent"}}


This is correct - just update the name to popover-parent to match our naming conventions

This was my updated tooltip code for reference:

{{#> tooltip pf-c-tooltip-modifier="pf-m-left"}} Tooltip (left) {{#*inline "tooltip-parent"}} {{#> button btnClass='pf-m-primary'}} button parent {{/button}} {{/inline}} {{/tooltip}}

matthewcarleton

Thanks for doing this John! I've added a few comments. I still have questions about a few things, and as you stated this is still a WIP so I think a deeper review is required once you've finished everything.

matthewcarleton · 2018-07-04T14:20:20Z

src/patternfly/components/Popover/styles.scss

+
+  &.pf-m-left &__arrow {
+    right: calc(-1 * var(--pf-c-popover__arrow--Size));
+    bottom: calc(50% - var(--pf-c-popover__arrow--Size));


I believe we want to keep all calc in the var declaration up top ^

matthewcarleton · 2018-07-04T14:23:25Z

src/patternfly/components/Popover/styles.scss

+    flex-direction: column;
+    justify-content: flex-start;
+    min-width: var(--pf-c-popover__content--MinWidth);
+    padding-top: var(--pf-c-popover__content--PaddingTop);


you can use shorthand here for padding

matthewcarleton · 2018-07-04T14:37:17Z

src/patternfly/components/Popover/styles.scss

+    z-index: var(--pf-c-popover__container--ZIndex);
+    display: flex;
+    width: var(--pf-c-popover__container--Width);
+    filter: var(--pf-c-popover__arrow--DropShadow);


it looks like there is an issue with the bottom tooltip with this approach. We lose the arrow when on a white background.

matthewcarleton · 2018-07-04T14:44:49Z

src/patternfly/components/Tooltip/styles.scss

+  --pf-c-tooltip__arrow--Size: var(--pf-global--arrow--BorderWidth);
+
+  // Content variables
+  --pf-c-tooltip__content--PaddingTop: var(--pf-global--spacer--lg);


the spacing for tooltips should be 32px according to the designs

matthewcarleton · 2018-07-04T14:52:13Z

src/patternfly/components/Popover/styles.scss

+
+  // Content variables
+  --pf-c-popover__content--MinWidth: 120px;
+  --pf-c-popover__content--PaddingTop: var(--pf-global--spacer--md);


same as the tooltip the design calls for 32px spacing here.

jlmitch5 · 2018-07-05T13:49:03Z

@dgutride thanks for figuring that out! latest commit adds those changes, which worked for me locally!

dgutride

handlebars changes look good

jgiardino · 2018-07-05T19:09:00Z

Thanks for adding these examples! I know you're still working on accessibility, but just wanted to throw some thoughts out there.

For both popovers and tooltips, you have the parent toggle come after the popover or tooltip. I was expecting that the parent toggle would precede the popover or tooltip in the DOM.

For the Popover, I think this component is more like a non-modal dialog, than like the inclusive toggletip example provided on inclusive-components.design, since we provide a Close action and would allow other interactive elements inside the popover (is that correct?). And if that's true, then the Dropdown component is probably the most comparable example to the Popover in terms of aria attributes. Both would have aria-haspopup, but instead of using role="menu" like we do for the dropdown menu, the Popover would use role="dialog" for the popover. Aside from those differences, the overall structure would be similar, with the toggle preceding the popover in the DOM order. This enables users who navigate using a keyboard to proceed forward to access the contents of the popover (unless we automatically handle focus for them).

For Tooltips, since these components would display based on hover/focus on the parent toggle, the question about the order in the DOM mainly centers around whether we want to use CSS to handle the display of Tooltips. If the tooltip is the next sibling of the toggle, then we could use just css to handle the display of tooltips. The only thing JS would be needed for is to provide the Esc key functionality to hide the tooltip.

dmiller9911

The only real changes I think that are important for React and other implementations, including a vanilla JS implementation, is to leave positioning out of the CSS.

dmiller9911 · 2018-07-10T14:58:33Z

src/patternfly/components/Popover/styles.scss

+
+  // Modifiers
+  &.pf-m-left &__container {
+    top: 50%;


I don't think this positioning makes sense to be in here. Since frameworks/JS will be responsible for positioning these elements I think it would make more sense to leave it out since it is just one more thing that we will need to overwrite. The modifiers applied for left, right, etc should only drive the caret. Most frameworks, including css/js only frameworks, will append the tooltip/popover to the body instead of keeping them inline with the target. This will allow them to be placed in areas that have overflow set to hidden.

This is outside the scope of this PR, but since @dmiller9911 mentioned the JS implementation, I wanted to share some thoughts.

My only concern about appending popovers to the body is that if we do, we need to manage focus so that users who use the keyboard to navigate (sighted or screen reader users) are able to shift focus from the toggle immediately to the popover contents. And then there's also the question of what happens when focus leaves a popover (e.g. Does it close automatically? If not, and the user leaves the popover then goes back, are you able to programmatically manage focus so that they go back to the open popover instead of the toggle?)? I realize some of these are design questions too.

Wouldn't most popovers act like modals in terms of needing an explicit action to open the popover via keyboard and once open capturing the keyboard focus and only returning the focus to the control when it closes? I don't know if the user should be forced into navigating through a popover if they don't want to.

Tooltips would display non-interactive text and therefore no focus is needed at all. Would aria-describedby be sufficient with a hidden node containing the tooltip text?

To address @dmiller9911's point. I think that for no JS tooltip/popover the current implementation makes sense. But when they are appended to the body and used with a lib (such as popperjs), then we would need to override the positioning and transforms. It's easily done but maybe additional modifiers that could be applied directly to the arrow to change it's orientation but not apply any positioning.

@christianvogt I am good with overriding is that is what is decided. It would be nice to remove the cascade here though with the child selector. This will either require us to use !important or make our own selector that targets the child. Further complicating the override. This could be done by removing container and moving all the styles up to pf-c-tooltip.

@dmiller9911 A lib like popperjs would apply the position via styles directly on the element which would override any css rules. I doubt you'd have to create new rules for your use case. But any positioning provided by pf would not be necessary with popperjs.

Wouldn't most popovers act like modals in terms of needing an explicit action to open the popover via keyboard and once open capturing the keyboard focus and only returning the focus to the control when it closes?

Copying @mcarrano on this one. I think popovers would be modal and not modeless, but I want to make sure designers are on the same page. If modal, then the issue of managing focus is probably easier to solve if we go with @dmiller9911's approach with the DOM.

I don't know if the user should be forced into navigating through a popover if they don't want to.

I agree, I expect the popover to only display based on explicit action by a user.

Tooltips would display non-interactive text and therefore no focus is needed at all. Would aria-describedby be sufficient with a hidden node containing the tooltip text?

Yes, I agree with this one too. But related to accessibility of tooltips, there are a couple of points worth noting:

In previous screen reader tests, I've noticed that if I had an aria attribute reference the id of a hidden element, the screen reader ignored it. So maybe it would be "hidden" using something like a sr-only class, but not hidden with the html attribute.

@jlmitch5, @LHinson I'm wondering if it makes sense to capture accessibility concerns related to tooltips in a separate issue. It might be worth setting up some test pages with variations to see what works and what doesn't in the context of a screen reader.

There's a new WCAG 2.1 item (1.4.13) that affects tooltips, where anything that becomes visible on hover or focus must be dismissible via the Esc key

Yes, the popover behaves like a modal. The user takes an explicit action to open the popover and it is closed either by a Close button inside the popover or by clicking away from it.

@dmiller9911 unfortunately, I can't excise the container dom element, because it is being used to separate the tooltip's content from the parents which is wrapped by the component block element.

That is ok. We will work around anything that we have to. It might work as is too. We will know more once we start implementing it.

christianvogt · 2018-07-11T03:31:10Z

src/patternfly/components/Popover/styles.scss

+  }
+
+  // Modifiers
+  &.pf-m-left &__container {


These rules should use >. Not that I condone a popover within a popover, but this rule could inadvertently affect other &__container's nested deeper in the tree and mess up the positioning of the popover if it isn't the same as the parent.

jgiardino · 2018-07-11T17:56:01Z

@jlmitch5 I captured accessibility requirements for Tooltips in a separate issue #456 so that we have more time to iterate and test for this component.
We can do the same for popovers if needed.

jlmitch5 · 2018-07-13T21:33:48Z

with a lot of help from @christianvogt, I've updated the pr to include an overlay component which wraps parent and the tooltip component itself. I've pushed that if anyone wants to check it out.

I'm going to update the popover to utilize this same pattern next week.

christianvogt · 2018-07-16T15:15:56Z

src/patternfly/components/Popover/styles.scss

+  --pf-c-popover__arrow--BoxShadow: var(--pf-global--BoxShadow);
+  --pf-c-popover__arrow--BackgroundColor: var(--pf-global--BackgroundColor--100);
+
+  $half-size: $pf-global--arrow--size / 2;


This can't be a sass variable but must be a css var because we give users the ability to override local css vars. Therefore the half size should be var(--pf-c-popover__arrow--Size) / 2

@andresgalante - do you agree with this assessment?

--pf-c-popover__arrow--Width: var(--pf-global--arrow--Width); // For positioning, divide arrow width by 2 --pf-c-popover__arrow--half-size: calc(var(--pf-c-popover__arrow--Width) / 2); --pf-c-popover__arrow--Margin: var(--pf-c-popover__arrow--half-size);

The half size must be calculated using the local variable --pf-c-popover__arrow--Width and not the global:

--pf-c-popover__arrow--Width: var(--pf-global--arrow--Width); // For positioning, divide arrow width by 2 --pf-c-popover__arrow--half-size: calc(var(--pf-c-popover__arrow--Width) / 2); --pf-c-popover__arrow--Margin: var(--pf-c-popover__arrow--half-size);

Good catch, updated

christianvogt · 2018-07-16T15:16:23Z

src/patternfly/components/Popover/styles.scss

+  --pf-c-popover--Margin: var(--pf-global--arrow--size);
+
+  // Arrow variables
+  --pf-c-popover__arrow--Width: var(--pf-global--arrow--size);


If an arrow is squared, it shouldn't have separate width and height variables but rather only a size.

christianvogt · 2018-07-16T15:18:38Z

src/patternfly/components/Tooltip/styles.scss

+  --pf-c-tooltip__arrow--BoxShadow: var(--pf-global--BoxShadow);
+  --pf-c-tooltip__arrow--BackgroundColor: var(--pf-global--BackgroundColor--100);
+
+  $half-size: $pf-global--arrow--size / 2;


same comment as popover

christianvogt · 2018-07-16T15:18:46Z

src/patternfly/components/Tooltip/styles.scss

+  --pf-c-tooltip__content--BackgroundColor: var(--pf-global--BackgroundColor--100);
+
+  // Arrow variables
+  --pf-c-tooltip__arrow--Width: var(--pf-global--arrow--size);


same comment as popover

christianvogt · 2018-07-16T15:51:02Z

src/patternfly/components/Tooltip/styles.scss

+
+    // necessary due to setting width
+    // this fixes pointer events using a hack
+    // unsure about browser compatibility of this hack


This comment needs more clarity.

Setting width: 100vw is necessary since the __container is absolutely positioned and we want to allow the content enough space to work in and have its layout flow. However, the __container width creates an invisible block where a user cannot click through. Therefore the __container is given pointer-events: none to allow clicking elements underneath. This negatively impacts the content and we need to enable pointer events on all children of the __container to allow the content to be interactive.

andresgalante

@jlmitch5 thanks for doing this PR, great work.

andresgalante · 2018-07-16T15:59:13Z

src/patternfly/components/Popover/styles.scss

+//   }
+// }
+
+.pf-c-popover {


@mcarrano @jlmitch5 why having a popover and a tooltip when they are basically the same component?

The styles may be identical, but is it possible to have a single component that has different interaction behavior and different contents that are allowed? My understanding is that the popover displays on click/Enter key and therefore can include interactive contents, whereas the tooltip displays on hover/focus and therefore can only display non-interactive contents.

andresgalante · 2018-07-16T16:01:43Z

src/patternfly/components/Popover/styles.scss

+  }
+
+  &.pf-m-top {
+    margin-bottom: var(--pf-c-popover--Margin);


I see the need for a single size declaration, but this name breaks the system. Therefore I'd just use --pf-c-popover__arrow--Width and write a good comment explaining whats happening here.

andresgalante · 2018-07-16T16:02:20Z

src/patternfly/components/Tooltip/docs/code.md

+| `.pf-c-tooltip` | `<div>` |  Creates a tooltip. |
+| `.pf-c-tooltip__arrow` | `.pf-c-tooltip` |  Creates an arrow pointing towards the element the tooltip describes. ** Required.** |
+| `.pf-c-tooltip__content` | `.pf-c-tooltip` |  Creates the body of the tooltip. ** Required.** |
+| `.pf-m-left` | `.pf-c-tooltip` and `.pf-c-overlay` | (Default) - Puts arrow to the left of the tooltip's content. |


why pf-c-overlay ?

@andresgalante - what should the developer do here specifically? would you recommend a different approach?

andresgalante · 2018-07-16T16:04:42Z

src/patternfly/utilities/variables.scss

@@ -68,6 +68,9 @@ $pf-global--danger-color--300:          $pf-color-red-400 !default;

 // Shadows

+$pf-global--DropShadow: drop-shadow(pf-size-prem(0) pf-size-prem(.7) pf-size-prem(2.1) rgba($pf-color-black-1000, .14)); // visually comparable to $pf-global--BoxShadow


DropShadow is not a css propierty. Can't we use the shadows we have defined?

andresgalante · 2018-07-16T16:07:14Z

src/patternfly/utilities/variables.scss

@@ -99,6 +102,7 @@ $fa-font-path: "./assets/fonts/webfonts" !default;
 $pf-global--image-path: "/assets/images" !default;

 // Spacers
+$pf-global--spacer--xxs: pf-size-prem(2px) !default;    // Color in the visuals


If we need to add more spacers on a global level lets first check with the designers. @mceledonia

andresgalante · 2018-07-16T16:07:41Z

src/patternfly/utilities/variables.scss

@@ -202,6 +206,9 @@ $pf-global--LineHeight--md: 1.5 !default;
 // List
 $global-ListStyle: disc outside !default;

+// Arrow size (used for tooltips, popovers, etc.)


I like this, we should create a story to use it on drop menus too.
I'd change px units to pf-size-prem(25px)

I see this as a different thing than the arrows that display in dropdowns. Wouldn't the arrows we use for dropdowns be sized relative to font size?

@andresgalante - based on @jgiardino's comment, is your direction to still use pf-size-prem(25px) ?

This isn't referring to the dropdown chevron size, it's referring to the white background arrow. So this size makes sense.
$pf-global--arrow--Width: pf-font-prem(25px);

andresgalante · 2018-07-16T16:10:44Z

src/patternfly/components/Popover/popover.hbs

+            </button>
+          </div>
+          <header class="pf-c-modal-box__header">
+            <h1 class="pf-c-modal-box__header-title" id="modal-title">


Why are we using modal classes within the popover, they are completely unrelated

I think the original suggestion was to use a similar structure to what we have for popovers, since they're very similar (e.g. both have a header, Close action, and contents), which I think makes sense. But I agree that we shouldn't have Modal classes here.

@andresgalante - what specifically should the developer do here?

change to pf-c-popover__header-title, style explicitly

JS concerns are not an issue here. React can work around anything else. Removing review.

add modifiers for properly padding preview panes add global arrow size variable

mattnolting · 2018-08-02T15:49:04Z

src/patternfly/components/Popover/popover.hbs

+      <div class="pf-c-popover {{direction}}">
+        <div class="pf-c-popover__arrow"></div>
+        <div class="pf-c-popover__content pf-c-modal-box" role="dialog" aria-modal="true" aria-labelledby="modal-title" aria-describedby="modal-description">
+          <div class="pf-c-modal-box__close">


change to pf-c-popover__close, style explicitly

mattnolting · 2018-08-02T15:54:25Z

src/patternfly/components/Popover/popover.hbs

+              <i class="fas fa-times"></i>
+            </button>
+          </div>
+          <header class="pf-c-modal-box__header">


change to pf-c-popover__header, style explicitly

mattnolting · 2018-08-02T15:54:45Z

src/patternfly/components/Popover/popover.hbs

+            </button>
+          </div>
+          <header class="pf-c-modal-box__header">
+            <h1 class="pf-c-modal-box__header-title" id="modal-title">


change to pf-c-popover__header-title, style explicitly

mattnolting · 2018-08-02T15:55:18Z

src/patternfly/components/Popover/popover.hbs

+              {{header}}
+            </h1>
+          </header>
+          <div class="pf-c-modal-box__body" id="modal-description">


change to pf-c-popover__body, style explicitly

mattnolting · 2018-08-02T16:10:49Z

src/patternfly/components/Popover/styles.scss

+  --pf-c-popover--MinWidth: 100px;
+  --pf-c-popover--MaxWidth: 300px;
+  --pf-c-popover--BoxShadow: var(--pf-global--BoxShadow);
+  --pf-c-popover--Margin: var(--pf-global--arrow--size);


--pf-c-popover--MinWidth: pf-size-prem(100px); --pf-c-popover--MaxWidth: pf-size-prem(300px); --pf-c-popover--BoxShadow: var(--pf-global--BoxShadow); // Set margin to arrow width for theming... --pf-c-popover--Margin: var(--pf-c-popover__arrow--Width);

mattnolting · 2018-08-02T16:37:58Z

src/patternfly/utilities/variables.scss

@@ -202,6 +206,9 @@ $pf-global--LineHeight--md: 1.5 !default;
 // List
 $global-ListStyle: disc outside !default;

+// Arrow size (used for tooltips, popovers, etc.)


This isn't referring to the dropdown chevron size, it's referring to the white background arrow. So this size makes sense.
$pf-global--arrow--Width: pf-font-prem(25px);

mattnolting · 2018-08-02T16:55:56Z

src/patternfly/components/Popover/styles.scss

+  &__arrow {
+    position: absolute;
+    width: var(--pf-c-popover__arrow--Size);
+    height: var(--pf-c-popover__arrow--Size);


// Popover arrow is square, sharing width value to prevent positioning errors width: var(--pf-c-popover__arrow--Width); height: var(--pf-c-popover__arrow--Width);

mattnolting · 2018-08-02T16:58:16Z

src/patternfly/components/Popover/styles.scss

+  &.pf-m-top > &__arrow {
+    bottom: -$half-size;
+    left: calc(50% - #{$half-size});
+  }


bottom: calc(var(--pf-c-popover__arrow-half-size) * -1); left: calc(50% - var(--pf-c-popover__arrow-half-size));

mattnolting · 2018-08-02T16:58:41Z

src/patternfly/components/Tooltip/docs/code.md

+| Attribute | Applied To | Outcome |
+| -- | -- | -- |
+| `role` or `aria` | `pf-c-tooltip` |  accessibility notes. |
+


Add any accessibility attributes

mattnolting · 2018-08-02T16:59:23Z

src/patternfly/components/Tooltip/styles.scss

+
+.pf-c-tooltip {
+  // Component variables
+  --pf-c-tooltip--MaxWidth: 200px;


--pf-c-tooltip--MaxWidth: pf-size-prem(200px);

Same naming conventions as popover

mattnolting · 2018-08-02T17:15:28Z

@christianvogt can you take a look at my comments and confirm?

christianvogt · 2018-08-02T18:00:26Z

@mattnolting since the arrow must be square, you still prefer to use Width over Size? is it because the name must match a css property?

Added a comment correcting the calculation of the half-size based off local var instead of global var.

The rest of your comments look good.

mattnolting · 2018-08-03T18:00:17Z

@christianvogt is the updated example what you expected? If so, I can merge. Any additional issues can be opened after..

jgiardino · 2018-08-03T19:33:56Z

src/patternfly/components/Popover/docs/code.md

+| `aria-labeledby="[id value of .pf-c-popover__header-title]"` | `.pf-c-popover` | Gives the popover an accessible name by referring to the element that provides the dialog title. **Required when .pf-c-popover__header-title is present** |
+| `aria-label="[title of popover]"` | `.pf-c-popover` | Gives the popover an accessible name. **Required when .pf-c-popover__header-title is _not_ present** |
+| `aria-describedby="[id value of applicable content]"` | `.pf-c-popover` | Gives the popover an accessible description by referring to the popover content that describes the primary message or purpose of the dialog. Not used if there is no static text that describes the popover. |
+| `aria-popover="true"` | `.pf-c-popover` | Tells assistive technologies that the windows underneath the current popover are not available for interaction. **Required**|


This looks like a Find/Replace mistake. 😉 I don't see aria-popover listed as an aria attribute.

I think you meant aria-modal="true" instead.

I opened #527 to track this one separately from this PR.

jgiardino · 2018-08-03T19:35:51Z

src/patternfly/components/Popover/docs/code.md

+| `aria-label="[title of popover]"` | `.pf-c-popover` | Gives the popover an accessible name. **Required when .pf-c-popover__header-title is _not_ present** |
+| `aria-describedby="[id value of applicable content]"` | `.pf-c-popover` | Gives the popover an accessible description by referring to the popover content that describes the primary message or purpose of the dialog. Not used if there is no static text that describes the popover. |
+| `aria-popover="true"` | `.pf-c-popover` | Tells assistive technologies that the windows underneath the current popover are not available for interaction. **Required**|
+| `aria-label="Close Dialog"` | `.pf-c-popover__close .pf-c-button` | Provides an accessible name for the close button as it uses an icon instead of text. **Required**|


@mcarrano Did you have a text string you wanted to use for the Close buttons on popovers? This looks like it was copied over from the Modal example, but I'm assuming we'd want something different for popovers.

@jgiardino Do we even need the label to reference the object? What if we just say "Close?"

That works for me. Would you want the same label for the ModalBox example? If so, we should open an issue to update that one, too.

Yes, I think so @jgiardino . We shouldn't assume users will know what we mean when we use words like "dialog" or "popover." Aren't popovers some kind of puff pastry? :)

But that would go nicely with our food theme: kebab, hamburger, toast...

I opened an issue to capture the text string changes for both this and the modal so that we can track that change separately from this PR: #525

jgiardino · 2018-08-03T19:43:36Z

src/patternfly/components/Popover/popover.hbs

-            {{> @partial-block}}
-          </div>
+<div class="Preview__overlay">
+  <div class="Preview__overlay-container {{direction}}">


This might be a @dgutride question, but for similar cases where we need to add styles to the preview area to accommodate components with absolute-positioned elements, we have been putting classes in examples/index.js.

We should only include classes for PF4 components in these files, since these files determine what code shows up in the code samples. But the classes you added here for the preview are for the workspace only, and are not something that we want consumers to copy when they copy the code samples.

agree here - let's get it in as it is if we can and file an issue to address this on the workspace (for me to handle) @jgiardino

Sounds good. I created this one: #526

jgiardino · 2018-08-03T19:51:57Z

src/patternfly/components/Popover/docs/code.md

+| `.pf-m-left` | `.pf-c-popover` | Puts arrow to the left of the popover's content. |
+| `.pf-m-right` | `.pf-c-popover` | Puts arrow to the right of the popover's content. |
+| `.pf-m-top` | `.pf-c-popover` | Puts arrow on top of the popover's content. |
+| `.pf-m-bottom` | `.pf-c-popover` | Puts arrow on the bottom of the popover's content. |


These last four rows are accurate. The class .pf-m-top is applied to the element with class .pf-c-popover, but I think all the other rows (except for the first) are inaccurate. For example, .pf-c-popover__arrow is applied to a <div> that's a child of .pf-c-popover, but it's not applied to .pf-c-popover directly.

I opened #527 to track these changes separately from this PR.

LHinson · 2018-08-04T11:53:19Z

@jgiardino thanks for adding those issues!
@christianvogt please file subsequent issues with any concerns/questions.
@mattnolting @dgutride I think we can move forward with a merge on this. please verify!

…ddTooltipComponent # Conflicts: # src/site/workspace.scss

dgutride requested changes Jul 4, 2018

View reviewed changes

matthewcarleton reviewed Jul 4, 2018

View reviewed changes

dgutride approved these changes Jul 5, 2018

View reviewed changes

dmiller9911 previously requested changes Jul 10, 2018

View reviewed changes

christianvogt reviewed Jul 11, 2018

View reviewed changes

jlmitch5 force-pushed the addTooltipComponent branch from f9ad46b to 14b86b7 Compare July 16, 2018 15:10

christianvogt suggested changes Jul 16, 2018

View reviewed changes

andresgalante requested changes Jul 16, 2018

View reviewed changes

John Mitchell added 2 commits July 16, 2018 13:15

updating globals for popovers and tooltips

9842af1

add modifiers for properly padding preview panes add global arrow size variable

add tooltip and popover components

0046f33

LHinson mentioned this pull request Jul 19, 2018

Tooltip #113

Closed

move overlay outside of components

a47ee16

mattnolting reviewed Aug 2, 2018

View reviewed changes

update tooltip and popovers based on feedback

ea916fb

jgiardino reviewed Aug 3, 2018

View reviewed changes

jgiardino mentioned this pull request Aug 3, 2018

text label for X close buttons #525

Closed

This was referenced Aug 3, 2018

Remove workspace classes from code samples in Popover/Tooltip #526

Closed

Popover/Tooltip documentation #527

Closed

Merge branch 'master' of github.com:patternfly/patternfly-next into a…

0daf1d8

…ddTooltipComponent # Conflicts: # src/site/workspace.scss

dgutride merged commit 63ee9c1 into patternfly:master Aug 6, 2018

		@@ -68,6 +68,9 @@ $pf-global--danger-color--300: $pf-color-red-400 !default;

		// Shadows

		$pf-global--DropShadow: drop-shadow(pf-size-prem(0) pf-size-prem(.7) pf-size-prem(2.1) rgba($pf-color-black-1000, .14)); // visually comparable to $pf-global--BoxShadow

add tooltip and popover components #442

add tooltip and popover components #442

Conversation

jlmitch5 commented Jul 3, 2018 • edited

patternfly-build commented Jul 3, 2018 • edited

christianvogt commented Jul 3, 2018

jlmitch5 commented Jul 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewcarleton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlmitch5 commented Jul 5, 2018

dgutride left a comment

Choose a reason for hiding this comment

jgiardino commented Jul 5, 2018

dmiller9911 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgiardino commented Jul 11, 2018

jlmitch5 commented Jul 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattnolting Aug 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andresgalante left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattnolting Aug 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattnolting commented Aug 2, 2018

christianvogt commented Aug 2, 2018

mattnolting commented Aug 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlmitch5 commented Jul 3, 2018 •

edited

patternfly-build commented Jul 3, 2018 •

edited

mattnolting Aug 2, 2018 •

edited

mattnolting Aug 2, 2018 •

edited