-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
Add Toggle implementation #159
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Max Desiatov <max@desiatov.com>
Weird issue: when clicking the “Check me!” checkbox, I get this error: “Fatal error: ToggleStyleKey must have a renderer-provided default value.” Somehow, the default environment values are not being provided. |
} | ||
|
||
public enum ToggleStyleKey: EnvironmentKey { | ||
public static var defaultValue: AnyToggleStyle { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the single defaultValue
provided for this key. Thus fatalError
is what we get, which needs to change for it to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the default environment includes a value for the key:
var environment = EnvironmentValues()
environment[ToggleStyleKey] = AnyToggleStyle(DefaultToggleStyle())
https://github.com/swiftwasm/Tokamak/pull/159/files#diff-6f90e40d15b14ddcb32508552c68db47R71
Ah, this may be related to the bug I've seen where |
Sorry for the delay, I've stumbled upon a different (but kinda related) bug that I'm finding hard to fix without proper stacktraces, so I'm thinking that the entrypoint/polyfills code in |
No worries! I’m fine waiting. |
This may be unblocked with the changes to environment in #170 |
Thanks, @carson-katri! The new version seems to be working well, with the caveat that the “I’m always checked!” checkbox can still be unchecked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you be able to resolve the conflict please?
Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you 🙌
I think it’s fine to merge this without the toggle style since that’s not super common on the web.