-
Notifications
You must be signed in to change notification settings - Fork 7
Add a Embed module #10
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
base: main
Are you sure you want to change the base?
Conversation
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.
In general, this PR makes sense
import Element.WithContext.Internal as Internal exposing (Attr(..), Element(..), run, runAttr) | ||
|
||
|
||
unwrap = |
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 missing documentation
type alias Placeholder context msg = | ||
InputInternal.Placeholder context msg |
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.
Why are you doing this? [not saying it's wrong, just curious about the reason]
= Option (context -> Input.Option value msg) | ||
|
||
|
||
runOption : a -> Option a value msg -> Input.Option value msg |
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.
Sorry to be nitpicky, but can you s/a/context/
@@ -0,0 +1,103 @@ | |||
module Element.WithContext.Embed exposing |
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.
Not sure if it's better to call it Embed
or Interop
? Probably Embed
is better
src/Element/WithContext.elm
Outdated
@@ -1,5 +1,5 @@ | |||
module Element.WithContext exposing | |||
( with, withAttribute, withDecoration, layout, layoutWith, element, attribute, attr, embed, embedAttr | |||
( with, withAttribute, withDecoration, layout, layoutWith, element, attribute, attr |
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 an alternative to #8 based on the embed-api branch by @miniBill
It exposes the 'run' functions directly in addition to the embed functions to deal with corner cases (mainly, having 2 different 'msg' types).