Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Paragraph - enable paragraph to have a custom content #566

Closed
4 tasks
daniele-zurico opened this issue Nov 2, 2023 · 7 comments
Closed
4 tasks

Paragraph - enable paragraph to have a custom content #566

daniele-zurico opened this issue Nov 2, 2023 · 7 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@daniele-zurico
Copy link
Contributor

daniele-zurico commented Nov 2, 2023

Currently, the paragraph component allows only to have a value however it should allow it to contain custom content something like:

<Paragragraph>
   <ImCustomComponent>
</Paragraph>

The current paragraph however is already used in production so we can't remove the value property the only thing we can do is to extend the use by adding this extra ability.
We need to make sure however that is you using the new function you not using the old one so something like this:

<Paragragraph value="im the value">
   <ImCustomComponent>
</Paragraph>

will not happen.

  • make the change in the src/abbreviate/Abbreviate.tsx adding the property
  • make sure that if children is passed you can't use value as well and vice-versa
  • make the change in the src/abbreviate/tests/Abbreviate.test.tsx adding the test to cover the use case
  • change all the storybook sections to add the documentation
@daniele-zurico daniele-zurico added the enhancement New feature or request label Nov 2, 2023
@daniele-zurico daniele-zurico added this to the 1.0.0 milestone Nov 2, 2023
@daniele-zurico
Copy link
Contributor Author

an Idea on how it can be implemented

type BaseProps = {
  test: string;
  a?: string;
  b?: string;
};

type PropsA = BaseProps & { a: string };
type PropsB = BaseProps & { b: string };

type Props = PropsA | PropsB;

function Component(props: Props) {
  if (props.a) {
    // do something
  }

  if (props.b) {
    // do something
  }

  return null;
} 

@zweertsk
Copy link

zweertsk commented Nov 5, 2023

This looks like arbitrary composition. Have the Paragraph accept a prop that is either of type 'value' or another component.

function Component({value, children}) {

    return (
        <Paragraph value={<AnotherComponent />} />
        <Paragraph value={"A string as p text"} />
    )
}

@jgonza16
Copy link
Collaborator

jgonza16 commented Nov 5, 2023

you suggest only one way with prop value. this approach makes the component less complex but i feel more natural the paragraph works like higher-order component. I think we will use this more.

<Paragraph>
  This is the simple custom-content of the
  <strong>
    paragraph
  </strong>
</Paragraph>

@kjzweerts you can review #567 and you get feedback.

@daniele-zurico
Copy link
Contributor Author

daniele-zurico commented Nov 5, 2023

hi @kjzweerts thanks for your comment and I think your point is valid however I do agree with @jgonza16 .... it will looks more natural to use the component like an html native component:

<Paragraph>content</Paragraph>

the only reason we want to leave value property there is because some other projects are already using the Paragraph and we don't want to introduce a breaking change.

On the other side can I ask you if you using the library? just out of curiosity

@zweertsk
Copy link

zweertsk commented Nov 6, 2023

On the other side can I ask you if you using the library? just out of curiosity

No I am not a user. I am in an Angular project and my react knowledge is starting to get vague 😅.
So I’m maybe looking to contribute to have best of both worlds.

I work for Sogeti and found this when browsing GitHub.

@daniele-zurico
Copy link
Contributor Author

absolutely @kjzweerts ... let me know when you want to get involved and in which way and we can discuss a ticket as starting point. Feel free to contact me on msTeam

@daniele-zurico
Copy link
Contributor Author

Closed by #567

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants