-
-
Notifications
You must be signed in to change notification settings - Fork 278
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
feat: on page load handler #980
Conversation
I haven't added the change files yet and I need to test the Mac implementation, which I'm the most uncertain about, but thought I'd put this out there for early feedback. |
I've added all three handlers and I believe addressed all concerns and comments. |
Co-authored-by: Amr Bashir <[email protected]>
I'm just going to remove the handler for navigation starting. |
LGTM but I will keep the PR open a while for @wusyong and @lucasfernog to give their approvals too |
LGTM but there's still a clippy error. |
I would consider using a single event function |
@wusyong There is a clippy error, but I'm not sure how you all would like it resolved. The error is due to new lints applied to old code, not anything that I added. The issue is a structure that is a unit class for some environment and not for others so default is used to create it, but using default to build a unit struct is now a clippy violation. I think the most straight forward is to add a lint exclusion, but it's possible you want a different solution. @lucasfernog I could make that change. |
@lucasfernog I made those changes to the public API. @wusyong I opted to just just allow the clippy violation, but could do something else. |
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
Checklist
Other information