ExpandableText: Adds the expandable text component previously known as truncate component. #39

dlabaj · 2023-09-21T19:34:51Z

Adds the expandableText component to react component groups.

https://expandedText.surge.sh

fhlavac · 2023-09-25T09:34:14Z

@dlabaj we should not bring Sass support as we decided to use JSS for all of your components. Can you please migrate the styling to JSS?

...y-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableTextExample.tsx

.../content/extensions/component-groups/examples/ExpandableText/ExpandableTextInlineExample.tsx

fhlavac · 2023-10-10T09:19:49Z

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md

+---
+section: extensions
+subsection: Component groups
+id: ExpandableText


Suggested change

id: ExpandableText

id: Expandable text

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md

packages/module/src/ExpandableText/ExpandableText.tsx

fhlavac · 2023-10-10T09:45:44Z

@dlabaj posted some comments, overall looks good. Thank you 🙂

...y-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableTextExample.tsx

fhlavac · 2023-10-11T14:10:03Z

...nt/extensions/component-groups/examples/ExpandableText/ExpandableTextCustomButtonExample.tsx

+const text = `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore
+et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea
+commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla
+pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est
+laborum.`;


Suggested change

const text = `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore

et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea

commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla

pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est

laborum.`;

const text = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore

et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea

commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla

pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est

laborum.';

Unfortunatly this change doesn't work since the text spans multiple lines.

fhlavac · 2023-10-11T14:10:18Z

...xtensions/component-groups/examples/ExpandableText/ExpandableTextCustomLinkButtonExample.tsx

+const text = `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore
+et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea
+commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla
+pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est
+laborum.`;


Suggested change

const text = `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore

et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea

commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla

pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est

laborum.`;

const text = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore

et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea

commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla

pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est

laborum.';

Tried the suggestion but we need the ` to allow the paragraph to run on multiple lines. Otherwise we would need to add '+ a the end of each line.

packages/module/src/ExpandableText/ExpandableText.tsx

fhlavac · 2023-10-11T14:14:40Z

packages/module/src/ExpandableText/ExpandableText.tsx

+  return inline ? (
+    <React.Fragment>
+      <span className={expandableTextClasses} dangerouslySetInnerHTML={html} {...mouseOverHandler} {...props} />
+      {!hideExpandText && textOverflow && (showText === false ? expandButton : collapseButton)}


Suggested change

{!hideExpandText && textOverflow && (showText === false ? expandButton : collapseButton)}

{!hideExpandText && textOverflow && (showText ? collapseButton : expandButton)}

fhlavac · 2023-10-11T14:16:48Z

packages/module/src/ExpandableText/ExpandableText.tsx

+  ...props
+}: ExpandableTextProps) => {
+  const classes = useStyles();
+  const expandableTextClasses = clsx(classes.expandableText, className, { [`is-inline`]: inline }, { [`is-block`]: !inline });


do we use is-block?
we could make these classes camelCase as well

fhlavac · 2023-10-11T14:17:43Z

packages/module/src/ExpandableText/ExpandableText.tsx

+
+const dangerousHtml = (html: string) => ({ __html: sanitizeHtml(html) });
+
+export interface ExpandableTextCustomButtonProps {


thinking if this additional interface is needed... why not just add expandToggle and collapseToggle to ExpandableTextProps ?

Because if they want to pass in a custom button they need to provide a collapse and expand button. However the custom button is optional in ExpandableTextProps. The documentation framework won't build if we put in a nested object in the props.

edonehoo

lmk what you think

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md

edonehoo · 2023-10-11T13:16:48Z

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md

+
+The **expandable text** component enables you to display long text to users via a tooltip. It builds off of the [tooltip component](https://www.patternfly.org/components/tooltip) to truncate UI text in an element and display the truncated text in a tooltip.
+
+### Basic ExpandableText component


Suggested change

### Basic ExpandableText component

### Basic expandable text

edonehoo · 2023-10-11T13:19:34Z

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md

+
+## Component usage
+
+The **expandable text** component enables you to display long text to users via a tooltip. It builds off of the [tooltip component](https://www.patternfly.org/components/tooltip) to truncate UI text in an element and display the truncated text in a tooltip.


Suggested change

The **expandable text** component enables you to display long text to users via a tooltip. It builds off of the [tooltip component](https://www.patternfly.org/components/tooltip) to truncate UI text in an element and display the truncated text in a tooltip.

The **expandable text** component enables you to truncate long text, which can be viewed in full after expansion.

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md

edonehoo · 2023-10-11T13:59:36Z

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md

+### ExpandableText component with inline text
+
+This is an example of a ExpandableText component with inline text. It will truncate the text and display the full text when the button is clicked. The button will be display inline with the text and will toggle the text.


Suggested change

### ExpandableText component with inline text

This is an example of a ExpandableText component with inline text. It will truncate the text and display the full text when the button is clicked. The button will be display inline with the text and will toggle the text.

### With an inline button

To style the button inline with the truncated text, rather than below, pass in `inline={true}`.

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md

edonehoo · 2023-10-11T14:42:03Z

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md

+
+import ExpandableText from "@patternfly/react-component-groups/dist/dynamic/ExpandableText";
+
+## Component usage


Suggested change

## Component usage

edonehoo · 2023-10-11T14:42:19Z

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md

+## Component usage
+
+The **expandable text** component enables you to truncate text and display the full text via a button. The button can be displayed inline with the text or below the text. The button can be customized to display custom text or a custom button.  Alternatively you can display the full text on hover.
+


Suggested change

## Examples

moving below the "intro" text

Hyperkid123 · 2023-10-12T06:53:43Z

packages/module/src/ExpandableText/ExpandableText.tsx

+      {button}
+    </Button>;
+
+  const expandButton = 


I am in favor of splitting the expand button into a separate component and providing it as a default prop to a new expandButton prop. Adding props API alongside a custom node element API is IMO a duplicate. If we split the expand button we give the consumer the ability to customize every aspect of the button.

They will have the ability to use a pre-defined variant, customize the props of the predefined variant by importing and changing its props, and use an entirely custom node if they choose to.

I don't think we would need to make an expandButton component if we go this route. They could use a PF Button and pass that to use. Only thing the button did internally for the component is control showing the state, however if we are not going to control the state then I don't see the reason to create an expandButton component.

Thoughts?

I don't think we would need to make an expandButton component if we go this route.

That is a valid point. Part of this is also checking if we actually need components like that. We have already identified several components that are "useless" and we will discontinue them from FEC. Maybe this component should be just a demo in some docs.

@fhlavac brought this up as well last week if we would need this component. I think having a demo in some docs would work. Does that sound good?

Regarding splitting the expand button into a separate component - In case we agree on keeping this component, I would say it's a good idea. The button component doesn't have to be (and probably should not be) a separate section in our docs, it can be still just part of ExpandableText docs.

Also agree that without the internal state, the component probably does not make sense to exist. So I would vote for either keeping the component with its internal state (use it or implement your own) or dropping it completely.

@Hyperkid123 Thoughts on keeping state in and keeping this component?

If we pull the button out we are going to have to take the state with it. Due to that I'm suggesting either getting rid of this component. I think a demo would be better.

If I'm not mistaken, we wouldn't have to move the state to the button, it could only be consumed as a boolean flag according to which it would display its corresponding variant. When clicked, it would then just call the function of the parent component, which would change the state.

Anyway, it's getting rid of this component and creating a demo is probably the best solution. Talked with Martin and he is for dropping as well.

Okay I'll close this PR, and get a demo put together somewhere.

Hyperkid123 · 2023-10-12T06:58:25Z

packages/module/src/ExpandableText/ExpandableText.tsx

+  const expandableTextClasses = clsx(classes.expandableText, className, { [`isInline`]: inline });
+  const trimmedText = text.substring(0, length);
+  const textOverflow = text.length > length;
+  const [ showText, setShowText ] = useState(false);


In general, we should try to have as little internal state management as possible. I would lift the expanded state out of this component.

It will simplify the component and the consumer will choose when and how to expand the text. We won't have to deal with cloning the buttons as that can be configured from the parent and with any other event. The logic can be also handled by completely different means.

Updated build:watch to build css and perform transform. Updated with comments. Fixed broken tests. fixed broken tests. fixed broken tests. Fixed export. Updated with review comments. Updated examples. Fixed import. Updated with correct documentation. Updated with review comments. Update packages/module/patternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com> Update packages/module/patternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com> Update packages/module/patternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com> Update packages/module/patternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com> Update packages/module/patternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableTextExample.tsx Co-authored-by: Filip Hlavac <50696716+fhlavac@users.noreply.github.com> Update packages/module/patternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com> Update packages/module/patternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com> Auto stash before checking out "origin/truncate"

dlabaj · 2023-10-19T12:58:23Z

Closing this PR in favor of doing a demo instead.

dlabaj requested a review from fhlavac September 21, 2023 19:34

dlabaj added the do not merge label Sep 21, 2023

dlabaj requested a review from wise-king-sullyman September 21, 2023 19:52

dlabaj changed the title ~~WIP feat(Truncate): Added the truncate component.~~ WIP feat(Truncate): Added the truncate component and SCSS support. Sep 22, 2023

dlabaj changed the title ~~WIP feat(Truncate): Added the truncate component and SCSS support.~~ WIP feat(Truncate): Adds the truncate component and SCSS support. Sep 22, 2023

dlabaj linked an issue Oct 5, 2023 that may be closed by this pull request

Migrate Truncate to component-groups #48

Closed

dlabaj changed the title ~~WIP feat(Truncate): Adds the truncate component and SCSS support.~~ ExpandableText: Adds the expandable text component previously known as truncate component. Oct 9, 2023

dlabaj added enhancement New feature or request and removed do not merge labels Oct 9, 2023

dlabaj requested a review from Hyperkid123 October 9, 2023 21:29

dlabaj self-assigned this Oct 9, 2023

fhlavac reviewed Oct 10, 2023

View reviewed changes

...y-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableTextExample.tsx Show resolved Hide resolved

fhlavac reviewed Oct 10, 2023

View reviewed changes

.../content/extensions/component-groups/examples/ExpandableText/ExpandableTextInlineExample.tsx Show resolved Hide resolved

fhlavac reviewed Oct 10, 2023

View reviewed changes

...atternfly-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableText.md Outdated Show resolved Hide resolved

fhlavac reviewed Oct 10, 2023

View reviewed changes

packages/module/src/ExpandableText/ExpandableText.tsx Outdated Show resolved Hide resolved

fhlavac reviewed Oct 10, 2023

View reviewed changes

packages/module/src/ExpandableText/ExpandableText.tsx Outdated Show resolved Hide resolved

fhlavac reviewed Oct 10, 2023

View reviewed changes

packages/module/src/ExpandableText/ExpandableText.tsx Outdated Show resolved Hide resolved

fhlavac reviewed Oct 10, 2023

View reviewed changes

packages/module/src/ExpandableText/ExpandableText.tsx Outdated Show resolved Hide resolved

dlabaj force-pushed the truncate branch 2 times, most recently from 22fb5c8 to 3d9e585 Compare October 10, 2023 20:57

dlabaj requested a review from edonehoo October 11, 2023 13:02

fhlavac reviewed Oct 11, 2023

View reviewed changes

...y-docs/content/extensions/component-groups/examples/ExpandableText/ExpandableTextExample.tsx Show resolved Hide resolved

fhlavac reviewed Oct 11, 2023

View reviewed changes

packages/module/src/ExpandableText/ExpandableText.tsx Show resolved Hide resolved

fhlavac reviewed Oct 11, 2023

View reviewed changes

edonehoo reviewed Oct 11, 2023

View reviewed changes

Hyperkid123 reviewed Oct 12, 2023

View reviewed changes

dlabaj added frontend migration hacktoberfest-accepted labels Oct 12, 2023

dlabaj force-pushed the truncate branch from e5bbb1c to 33c7824 Compare October 18, 2023 14:36

dlabaj closed this Oct 19, 2023

dlabaj mentioned this pull request Dec 13, 2023

Create code sand box demo for Expandable Text patternfly/patternfly-react#9888

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ExpandableText: Adds the expandable text component previously known as truncate component. #39

ExpandableText: Adds the expandable text component previously known as truncate component. #39

dlabaj commented Sep 21, 2023 •

edited

Loading

fhlavac commented Sep 25, 2023

fhlavac Oct 10, 2023

dlabaj Oct 11, 2023

fhlavac commented Oct 10, 2023

fhlavac Oct 11, 2023

dlabaj Oct 18, 2023

fhlavac Oct 11, 2023

dlabaj Oct 11, 2023

fhlavac Oct 11, 2023

fhlavac Oct 11, 2023

fhlavac Oct 11, 2023 •

edited

Loading

dlabaj Oct 11, 2023 •

edited

Loading

edonehoo left a comment

edonehoo Oct 11, 2023

edonehoo Oct 11, 2023

edonehoo Oct 11, 2023

edonehoo Oct 11, 2023

edonehoo Oct 11, 2023

edonehoo Oct 11, 2023

Hyperkid123 Oct 12, 2023

dlabaj Oct 18, 2023 •

edited

Loading

Hyperkid123 Oct 18, 2023

dlabaj Oct 18, 2023 •

edited

Loading

fhlavac Oct 18, 2023 •

edited

Loading

dlabaj Oct 18, 2023

dlabaj Oct 18, 2023

fhlavac Oct 19, 2023 •

edited

Loading

dlabaj Oct 19, 2023

Hyperkid123 Oct 12, 2023

dlabaj commented Oct 19, 2023

	{!hideExpandText && textOverflow && (showText === false ? expandButton : collapseButton)}
	{!hideExpandText && textOverflow && (showText ? collapseButton : expandButton)}


		const dangerousHtml = (html: string) => ({ __html: sanitizeHtml(html) });

		export interface ExpandableTextCustomButtonProps {


		The expandable text component enables you to display long text to users via a tooltip. It builds off of the [tooltip component](https://www.patternfly.org/components/tooltip) to truncate UI text in an element and display the truncated text in a tooltip.

		### Basic ExpandableText component


		## Component usage

		The expandable text component enables you to display long text to users via a tooltip. It builds off of the [tooltip component](https://www.patternfly.org/components/tooltip) to truncate UI text in an element and display the truncated text in a tooltip.

		### ExpandableText component with inline text

		This is an example of a ExpandableText component with inline text. It will truncate the text and display the full text when the button is clicked. The button will be display inline with the text and will toggle the text.


		import ExpandableText from "@patternfly/react-component-groups/dist/dynamic/ExpandableText";

		## Component usage

ExpandableText: Adds the expandable text component previously known as truncate component. #39

ExpandableText: Adds the expandable text component previously known as truncate component. #39

Conversation

dlabaj commented Sep 21, 2023 • edited Loading

fhlavac commented Sep 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhlavac commented Oct 10, 2023

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

fhlavac Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

dlabaj Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

edonehoo 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

dlabaj Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlabaj Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

fhlavac Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhlavac Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlabaj commented Oct 19, 2023

dlabaj commented Sep 21, 2023 •

edited

Loading

fhlavac Oct 11, 2023 •

edited

Loading

dlabaj Oct 11, 2023 •

edited

Loading

dlabaj Oct 18, 2023 •

edited

Loading

dlabaj Oct 18, 2023 •

edited

Loading

fhlavac Oct 18, 2023 •

edited

Loading

fhlavac Oct 19, 2023 •

edited

Loading