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

Docs: Add 'Change Event Callback' pattern doc #33216

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Mitch-At-Work
Copy link
Contributor

Implementation of Change Event pattern to document avoiding potential breaking changes and recommended architecture.

@fabricteam
Copy link
Collaborator

📊 Bundle size report

✅ No changes found

Copy link
Contributor

@mltejera mltejera left a comment

Choose a reason for hiding this comment

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

Some brief typos and a question, feel free to disregard if you disagree

🤜🤛

Copy link

github-actions bot commented Nov 7, 2024

📊 Bundle size report

✅ No changes found

Copy link

github-actions bot commented Nov 7, 2024

Pull request demo site: URL

@Mitch-At-Work Mitch-At-Work marked this pull request as ready for review November 7, 2024 21:25
@Mitch-At-Work Mitch-At-Work requested review from a team as code owners November 7, 2024 21:25
* @param data - A data object with relevant information,
* such as open value and type of interaction that created the event
*/
// eslint-disable-next-line @nx/workspace-consistent-callback-type -- can't change type of existing callback
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a deprecated callback type. What about use an example that uses the new callback type? For example TagPicker onOpenChange?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see the first example is all taken from dialog. So maybe just add a comment somewhere here pointing to a component that uses the new type like TagPicker

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch thank you! I'll replace this with the latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants