Patternfly-react: [PF4] [Popover] Popover title bottom margin too big

[jiridostal]

All our mockups show that popover title margin is smaller, can we please revisit this?

Now:

Expected:

@katierik

[katierik]

Key issue is adding a table inside the popover.

[tlabaj]

cc @mcoker @kybaker

[mcoker]

+ @mceledonia we use a 24px/large spacer below the popover title. Should we consider changing that or offering a variation for a smaller spacer?

This is also something consumers can override in their application, either globally, as a one-off, or by defining their own variant overriding the --pf-c-popover--c-title--MarginBottom variable to use a different spacer, like --pf-global--spacer--md or --pf-global--spacer--sm

[katierik]

For a Red Hat-made product, using Patternfly, would the recommendation be to override or to use the spacing that is included?

Perhaps a question to ponder for @LHinson @mcarrano @rachael-phillips

[mceledonia]

I would be in favor of dropping --pf-c-popover--c-title--MarginBottom down to --pf-global--spacer--md across the board. Is the issue here that a table doesn't fit at all, or that too much space between the header/body makes for an awkward look? I did think we had some flexibility in terms of popover size but it's been a while!

[katierik]

Its the awkward look :) The popover extends to meet the size a-ok

[mceledonia]

I'd drop it down from 24 to 16 under the header (..spacer--lg -> ..spacer--md) for the component, I don't think there's a strong reason to have a modifier or both, and I agree it is too large! @katierik @mcoker

[rachael-phillips]

@mcoker can we move this over to the Core repo? cc @jiridostal

[mcoker]

@rachael-phillips https://github.com/patternfly/patternfly-next/pull/2381 was already merged, so the change @mceledonia mentioned is updated in core. If the core update will work for this use case, I think we can close this issue either now or after react updates to core v2.36.2 and we verify the new spacing will work.

[rachael-phillips]

@jiridostal does the fix in patternfly/patternfly-next#2381 work for your use case?

[katierik]

Works for the design

[jiridostal]

@rachael-phillips Looks good! Thanks

[tlabaj]

Closing this issue since it was resolved in core.