feat(Text): add text to Accordion, Button, Checkbox, Combobox, Contentswitcher

[dakahn]

Closes #9462

[netlify]

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: acc7ec5

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/6148ab9a4b33ad0008384b2b

😎 Browse the preview: https://deploy-preview-9493--carbon-react-next.netlify.app

[tay1orjones]

🎉

[joshblack]

Looking great! Just a couple of usage notes/questions

[joshblack]

I think a big thing for the conversion will be any text that we accept as props that the user cannot control rendering of will need to be wrapped in a Text component. If they can supply children or configure the nod already, then we don't need to convert that part of a component. Curious what you all think after going through these five components 🤔

[netlify]

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: acc7ec5

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/6148ab9a4548e10007d1a511

😎 Browse the preview: https://deploy-preview-9493--carbon-elements.netlify.app

[netlify]

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: acc7ec5

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/6148ab9a09147300080d7f01

😎 Browse the preview: https://deploy-preview-9493--carbon-components-react.netlify.app

[dakahn]

@joshblack that makes sense to me!

[joshblack]

Not sure if you're still looking for a review from me, but I think the change left is in AccordionItem to wrap the title in a Text and to not wrap the content in a Text

[joshblack]

@dakahn saw the re-review request, I think my last review comment is here: #9493 (review)

[tay1orjones]

@joshblack fixed the accordion item 👍

[wajnberg]

Hi all
I just tested Button with TextDirection
I added this story (in Button-story.js)

export const WithTextDir = () => {
  return <TextDirection dir="auto"><Button kind="ghost">שלום!!</Button></TextDirection>;	
};

But I get:

It should be !!שלום
Am I missing anything ?

[wajnberg]

Ok I checked all the 5 components:
This is what I found:

• Button is not working (see above)
• Accordion is OK
• Checkbox is OK
• Combobox is partially OK. It is working for title and helper text. But what about the options themselves ?
  
• Contentsswitcher is not working
  

I further looked at the code you committed and I can actually see that no code has been changed for Button (apart from a cosmetic change ) and ContentSwitcher.

Thank you for your feedback

[tay1orjones]

Thanks for taking a look at this! I think you’ve highlighted a few gaps in documentation that we should address.

Button

It’s my understanding that for any component accepting children as a prop that is not explicitly a string, we’ll be recommending the use of the Text composed within the component. For instance, Button accepts children as a prop, but this prop can be things other than text (icons, etc). Based on this, the following updated snippet returns the text in RTL as you expect:

export const ButtonExample = () => {
  const buttonText = 'שלום!!';
  return (
    <Button kind="ghost">
      <Text>{buttonText}</Text>
    </Button>
  );
};

Listbox - ComboBox

For any component using the ListBox (Dropdown, ComboBox, MultiSelect, FilterableMultiSelect), you'll need to use the itemToElement function prop to render text in the proper direction through the downshift library these components use under the hood. The following updated snippet returns the option-1 text in RTL as you expect:

export const DropdownExample = () => {
  const items = [
    {
      id: 'option-0',
      text: 'Lorem, ipsum dolor sit amet consectetur adipisicing elit.',
    },
    {
      id: 'option-1',
      text: 'שלום!!',
    },
  ];
  return (
    <div style={{ width: 400 }}>
      <Dropdown
        id="default"
        titleText="Dropdown label"
        helperText="This is some helper text"
        label="Dropdown menu options"
        items={items}
        itemToElement={(item) => <Text>{item.text}</Text>}
      />
    </div>
  );
};

ContentSwitcher

The same general idea applies to ContentSwitcher.

export const ContentSwitcherExample = () => {
  const switchText = 'שלום!!';
  return (
    <ContentSwitcher onChange={() => {}}>
      <Switch name="one" text={<Text>{switchText}</Text>} />
      <Switch name="two" text="Second section" />
      <Switch name="three" text="Third section" />
    </ContentSwitcher>
  );
};

The above works, but it throws a prop-type warning. The text prop-type definition is configured to accept a string only. This will need to be updated to accept a node or similar to avoid the proptype warning.

[wajnberg]

Hello @tay1orjones ,

Thank you for your explanations.
It's much clearer now :-)