feat(Popper): add width props, remove popperMatchesTriggerWidth by kmcfaul · Pull Request #8724 · patternfly/patternfly-react

kmcfaul · 2023-02-20T21:23:56Z

What: Closes #8682

Adds width, minWidth, maxWidth to Popper.
Sets the default of minWidth to trigger (popper will grow to trigger width at minimum but will still grow to fit content).
Removes popperMatchesTriggerWidth (minWidth now covers this by default, and all width props can be set to trigger to match the trigger width).

kmcfaul · 2023-02-20T21:25:06Z

Codemod: patternfly/pf-codemods#286

patternfly-build · 2023-02-20T21:32:22Z

Preview: https://patternfly-react-pr-8724.surge.sh

A11y report: https://patternfly-react-pr-8724-a11y.surge.sh

kmcfaul · 2023-02-22T19:38:39Z

Removed several instances of minWidth="revert" that replaced popperMatchesTriggerWidth={false} because the default minWidth will satisfy why the flag was added (as it no longer fixes the width to the trigger).

Added minWidth props to several components to allow customization of the default rather than enforcing a revert or the default. Popover was not included as it has its own minWidth and maxWidth props, and OverflowTab as it is not directly used.

packages/react-core/src/helpers/Popper/Popper.tsx

mcoker

Noticing some glitchy text on truncated table header text with tooltips.

Mar-03-2023.15-51-03.mp4

Also anywhere we're setting menu widths, we shouldn't need to do that now, unless that's part of the thing we're aiming to show in the example/demo -

patternfly-react/packages/react-core/src/demos/ComposableMenu/examples/ComposableActionsMenu.tsx

Line 74 in 60f2694

'--pf-c-menu--Width': '300px'

packages/react-core/src/next/components/Select/Select.tsx

packages/react-table/src/components/Table/ActionsColumn.tsx

kmcfaul · 2023-03-07T18:01:17Z

@mcoker The table tooltip issue you were seeing was resolved in another PR, now that this is rebased it should be fixed here once the build finishes.

kmcfaul · 2023-03-07T19:56:06Z

@nicolethoen I'm unsure why this PR is failing a timestamp test as it shouldn't be impacted by these changes. Any thoughts?

tlabaj · 2023-03-07T22:33:05Z

@nicolethoen I'm unsure why this PR is failing a timestamp test as it shouldn't be impacted by these changes. Any thoughts?

Another PR was merges that is now causing this issue. I put up a PR to revert it.

tlabaj

Katie. once you rebase this PR the timestamp test issue should be resolved.

tlabaj

Looks good. just one small suggestion.

packages/react-core/src/next/components/Select/Select.tsx

tlabaj

Can you also remove the minWidth on the DropdownNext kebab example. It should no longer be needed.

I am actually wondering if we need the minWidth prop anymore. It was specifically added for the kebab use case. Menu does not expose a minWidth prop.

cc @mcoker

tlabaj · 2023-03-10T18:54:31Z

packages/react-core/src/next/components/Select/Select.tsx

 import { css } from '@patternfly/react-styles';
 import { Menu, MenuContent, MenuProps } from '../../../components/Menu';
-import { Popper } from '../../../helpers/Popper/Popper';
+import { Popper, PopperProps } from '../../../helpers/Popper/Popper';


Do you think the Select still needs a minwidth prop?.

Select (and Dropdown) still have access to the minWidth prop via popperProps that get spread directly to Popper.

I was just wondering if it would just be spread with popperProps. For The next Dropdown it looks like minWidth was removed on line 26. We should probably be consistent wit the two components.

Oh yeah forgot about that. I'll remove it from Select as well

srambach

Looks ok to me 👍

tlabaj

LGTM

patternfly-build · 2023-03-15T19:54:22Z

Your changes have been released in:

@patternfly/react-code-editor@5.0.0-alpha.34
@patternfly/react-core@5.0.0-alpha.34
@patternfly/react-docs@6.0.0-alpha.37
demo-app-ts@5.0.0-alpha.17
@patternfly/react-table@5.0.0-alpha.34

Thanks for your contribution! 🎉

kmcfaul force-pushed the popper-widths branch from 4477ecc to e97ce0e Compare February 20, 2023 22:49

tlabaj requested a review from thatblindgeye February 21, 2023 00:27

tlabaj added the css review label Feb 21, 2023

tlabaj requested a review from mcoker February 21, 2023 00:27

gitdallas mentioned this pull request Feb 21, 2023

Popper - popperMatchesTriggerWidth replaced with minWidth patternfly/pf-codemods#286

Closed

kmcfaul force-pushed the popper-widths branch from e71785d to 7136883 Compare February 22, 2023 19:35

thatblindgeye approved these changes Feb 22, 2023

View reviewed changes

packages/react-core/src/helpers/Popper/Popper.tsx Show resolved Hide resolved

mcoker reviewed Mar 3, 2023

View reviewed changes

packages/react-core/src/next/components/Select/Select.tsx Outdated Show resolved Hide resolved

packages/react-table/src/components/Table/ActionsColumn.tsx Outdated Show resolved Hide resolved

kmcfaul force-pushed the popper-widths branch from baa289a to ffb84d5 Compare March 7, 2023 17:58

tlabaj reviewed Mar 8, 2023

View reviewed changes

kmcfaul force-pushed the popper-widths branch from 6b07d76 to 8f0e1b9 Compare March 8, 2023 14:56

tlabaj requested changes Mar 8, 2023

View reviewed changes

packages/react-core/src/next/components/Select/Select.tsx Outdated Show resolved Hide resolved

tlabaj reviewed Mar 8, 2023

View reviewed changes

tlabaj requested a review from srambach March 10, 2023 16:18

tlabaj reviewed Mar 10, 2023

View reviewed changes

nicolethoen approved these changes Mar 13, 2023

View reviewed changes

kmcfaul added 8 commits March 14, 2023 14:30

feat(Popper): add width props, remove popperMatchesTriggerWidth

2e77815

update to fit-content

ce578dd

update to revert instead

d529018

update snap

0f22587

remove revert minWidth from examples, add minWidth to props

cf70c53

use var

8eab61b

pr feedback

e44836a

update some snaps

b0e7b75

kmcfaul added 4 commits March 14, 2023 14:30

add partial, remove dropdown-next minWidth

c4ab71a

update for removal

d3c81c5

forgot a removal

18e7a57

remove minwidth from select

cb5367a

kmcfaul force-pushed the popper-widths branch from a0b1c4b to cb5367a Compare March 14, 2023 18:31

remove widths

aad867e

srambach approved these changes Mar 15, 2023

View reviewed changes

tlabaj reviewed Mar 15, 2023

View reviewed changes

tlabaj approved these changes Mar 15, 2023

View reviewed changes

tlabaj merged commit ec65022 into patternfly:v5 Mar 15, 2023

Conversation

kmcfaul commented Feb 20, 2023

Uh oh!

kmcfaul commented Feb 20, 2023

Uh oh!

patternfly-build commented Feb 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kmcfaul commented Feb 22, 2023

Uh oh!

Uh oh!

mcoker left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kmcfaul commented Mar 7, 2023

Uh oh!

kmcfaul commented Mar 7, 2023

Uh oh!

tlabaj commented Mar 7, 2023

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj Mar 10, 2023

Choose a reason for hiding this comment

Uh oh!

kmcfaul Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

tlabaj Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

kmcfaul Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

srambach left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

patternfly-build commented Mar 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

patternfly-build commented Feb 20, 2023 •

edited

Loading

mcoker left a comment •

edited

Loading