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

DOM: support onClose event in dialog element #55

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

Hi-Angel
Copy link
Contributor

@Hi-Angel Hi-Angel commented Nov 8, 2024

@pete-murphy
Copy link
Member

Hi, thanks for the PR! The Generated module is output from this: https://github.com/purescript-react/purescript-react-basic-dom/blob/main/codegen/consts.js. Could you move this change to that file instead and run the codegen script?

@Hi-Angel
Copy link
Contributor Author

Hi-Angel commented Nov 8, 2024

Hi, thanks for the PR! The Generated module is output from this: https://github.com/purescript-react/purescript-react-basic-dom/blob/main/codegen/consts.js. Could you move this change to that file instead and run the codegen script?

Thank you, sure! But how do I regenerate the "Generated.pus" file afterwards?

In fact, I've seen that the file is generated, but I waited for someone to suggest how to make the proper change, because the header of the file has no mention how would one generate it 😅

@pete-murphy
Copy link
Member

Looks like you should be able to do

cd codegen
npm install
node index.js

@Hi-Angel Hi-Angel force-pushed the support-dialog-onclose branch from 84a704c to 19b0082 Compare November 8, 2024 16:34
@Hi-Angel
Copy link
Contributor Author

Hi-Angel commented Nov 8, 2024

@pete-murphy thank you, done!

I also made a commit that adds an explanation to the header about how to regenerate files, so in the future hopefully contributions from new users will be simpler 😊

Please review

@pete-murphy pete-murphy merged commit a0ed953 into purescript-react:main Nov 8, 2024
1 check passed
@pete-murphy
Copy link
Member

pete-murphy commented Nov 8, 2024

Thanks again! Released as part of v7.0.0 (which includes a breaking change to defaultChecked type: https://github.com/purescript-react/purescript-react-basic-dom/releases/tag/v7.0.0)

Updating the registry here: purescript/registry#501

@Hi-Angel
Copy link
Contributor Author

Hi-Angel commented Nov 8, 2024

@pete-murphy oh, thank you! But what do you think about this suggestion? Would be cool to have it included as well 😊

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.

2 participants