Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fixed Hint message type #3146

Merged
merged 1 commit into from
Jun 25, 2024
Merged

Fixed Hint message type #3146

merged 1 commit into from
Jun 25, 2024

Conversation

adids1221
Copy link
Contributor

Description

After changing Hint message prop type in this PR which became a bit lossy type, changing the message type to be ContentType more strict type.

Changelog

Hint message prop type fix.

Additional info

None

@adids1221
Copy link
Contributor Author

@Inbal-Tish we can discuss about the comment in the old PR when the user passing null as Andrew mentioned there.

@Inbal-Tish
Copy link
Collaborator

Inbal-Tish commented Jun 24, 2024

@Inbal-Tish we can discuss about the comment in the old PR when the user passing null as Andrew mentioned there.

Ok. Why do we want to support null? The massage prop is rendered in a Text component, why should we let the user pass null or number or boolean for that matter?

@Inbal-Tish Inbal-Tish assigned adids1221 and unassigned Inbal-Tish Jun 24, 2024
@adids1221
Copy link
Contributor Author

adids1221 commented Jun 24, 2024

I think the only case using null is when the user has some state with the message and sometime the state is set to null

I can think of 2 use cases:

  1. there is a state that the user set to null sometime which isn't right.
  2. the user is passing Text element which is and ReactElement with null value inside - Andrew case.

@adids1221 adids1221 assigned Inbal-Tish and unassigned adids1221 Jun 24, 2024
@Inbal-Tish
Copy link
Collaborator

Inbal-Tish commented Jun 25, 2024

I think the only case using null is when the user has some state with the message and sometime the state is set to null

I can think of 2 use cases:

  1. there is a state that the user set to null sometime which isn't right.
  2. the user is passing Text element which is and ReactElement with null value inside - Andrew case.

Not our business to fix. If the user doesn't want to render a message don't pass the prop, if using react element it's the user's responsibility to pass it correctly (check the value before passing the element, for example:
massage={sometext && <Text>someText</Text>}

@Inbal-Tish Inbal-Tish assigned adids1221 and unassigned Inbal-Tish Jun 25, 2024
@Inbal-Tish Inbal-Tish added the Important for Next Release PR that must be included in the release version label Jun 25, 2024
@adids1221 adids1221 merged commit 32049ef into master Jun 25, 2024
1 check passed
@adids1221 adids1221 deleted the infra/Hint_content_type branch June 25, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Important for Next Release PR that must be included in the release version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants