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

Update toast to always have up to date messages state #2669

Closed
wants to merge 2 commits into from

Conversation

omerts
Copy link

@omerts omerts commented Mar 2, 2022

Fixes bug #2668

Copy link
Member

@melloware melloware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me just wasnt' sure if PrimeTek had it in the state for a reason. But your fix looks good to me.

@omerts
Copy link
Author

omerts commented Mar 3, 2022

@melloware I presume that he had it on state in order to cause renders when this state changes, so instead I used forceUpdate for rendering on changes.

@melloware
Copy link
Member

Excellent @omerts Do you see my suggestion about adding a replace method as well like Mssages has so you can call it and it will call clear() and then show() for the new message?

public replace(message: ToastMessageType): void;

@omerts
Copy link
Author

omerts commented Mar 6, 2022

@melloware done :)

@melloware
Copy link
Member

melloware commented Mar 7, 2022

Can you also update the .d.tsfile to list this new replace method. Thanks!

public replace(message: ToastMessageType): void;

Copy link
Member

@melloware melloware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment about the TypeScript definition update.

@mertsincan mertsincan added the Status: Pending Review Issue or pull request is being reviewed by Core Team label Mar 29, 2022
@melloware
Copy link
Member

There are merge conflicts to fix.

@mertsincan mertsincan added Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated. and removed Status: Pending Review Issue or pull request is being reviewed by Core Team labels Apr 4, 2022
@melloware
Copy link
Member

@omerts do you mind updating your fixes?

@melloware
Copy link
Member

This is fixed now that hooks are being used: https://codesandbox.io/s/primereact-test-forked-5zuel3

@melloware melloware closed this Apr 10, 2022
@omerts
Copy link
Author

omerts commented May 17, 2022

@melloware Sorry for not getting back to this. Happy to know it is fixed, you are awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants