Feat(DescriptionList): 6866 set width of term column

[gitdallas]

What: Closes #6866

Additional issues:

[patternfly-build]

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

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

[nicolethoen]

Perhaps it'd be worth updating the other horizontal example to use a checkbox to apply a custom width when checked since it's only changing one prop.

[gitdallas]

@nicolethoen - just noticed your top level comment about a checkbox, will address that soon

[tlabaj]

Perhaps it'd be worth updating the other horizontal example to use a checkbox to apply a custom width when checked since it's only changing one prop.

@nicolethoen Maybe more than a checkbox is warranted, it would be ideal if the consumer can set the width. Most of our examples with checkboxes are to apply modifiers.

Tooltip has something that is more than modifying a boolean in an example
https://patternfly-react.surge.sh/components/tooltip#options

cc @mcarrano

[gitdallas]

@tlabaj - converted to a NumberInput

[tlabaj]

@gitdallas I don't think we have a good way to add anything other than a checkbox right now without having the control look like it is part of the component. I this case I think it is best to have a separate example to show the functionality.
An issue has been opened in design so we can update our example to better allow for adding controls.

[gitdallas]

@tlabaj - reverted back to separate examples

[mcoker]

This looks great, though does it support the breakpoint modifiers? https://codesandbox.io/s/stoic-mopsa-twtm0v?file=/style.css

[gitdallas]

This looks great, though does it support the breakpoint modifiers? https://codesandbox.io/s/stoic-mopsa-twtm0v?file=/style.css

not sure, basically just allowing the consumer to overwrite the --pf-c-description-list__term--width value. they pass the termWidth.

style.setProperty(termWidthToken.name, termWidth);

i'm not sure where they come from or what it means if they aren't there, but I only see a react-token for that main term_width which is given a value of 12ch

[mcoker]

@gitdallas ah ok, I'm assuming they don't show up in react tokens since we don't define them with an initial value. We just use them in a fallback stack --pf-c-description-list__term--width at different breakpoints. You can see how they're used here: https://unpkg.com/browse/@patternfly/patternfly@4.183.1/components/DescriptionList/description-list.css

I imagine termWidth would be an object that supports default, sm, md, etc keys to define the size(s), similar to maxWidths and minWidths in the gallery component

[gitdallas]

@nicolethoen @mcoker - see latest https://patternfly-react-pr-7061.surge.sh/components/description-list , i added modifiers for the width breakpoints.. it doesn't seem to have any affect. you can see they are applied

[nicolethoen]

I think you're updating the wrong breakpoint modifier.
Your various breakpoints should be setting the following CSS vars:

  --pf-c-description-list--m-horizontal__term--width (this is the default width)
  --pf-c-description-list--m-horizontal__term--width-on-sm
  --pf-c-description-list--m-horizontal__term--width-on-md
  --pf-c-description-list--m-horizontal__term--width-on-lg
  --pf-c-description-list--m-horizontal__term--width-on-xl
  --pf-c-description-list--m-horizontal__term--width-on-2xl

[gitdallas]

I think you're updating the wrong breakpoint modifier. Your various breakpoints should be setting the following CSS vars:

  --pf-c-description-list--m-horizontal__term--width (this is the default width)
  --pf-c-description-list--m-horizontal__term--width-on-sm
  --pf-c-description-list--m-horizontal__term--width-on-md
  --pf-c-description-list--m-horizontal__term--width-on-lg
  --pf-c-description-list--m-horizontal__term--width-on-xl
  --pf-c-description-list--m-horizontal__term--width-on-2xl

does it matter that there is no react token for these? just hard code them in?

edit: i do see that is being done in another place so i'll go ahead with that.

[mcoker]

Looks great!!! Just one nit about the docs.

[mcoker]

Left a comment re: the docs.

Also unrelated I noticed the enableFlip prop docs are missing a period between "necessary If" and at the end.

If true, tries to keep the popover in view by flipping it if necessary If the position is set to 'auto', this prop is ignored

[tlabaj]

looks good other than the little issue.

[mcoker]

Nice!!

[mturley]

Thanks so much everyone! This is perfect