-
Notifications
You must be signed in to change notification settings - Fork 21
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
fix: Improve form inputs #157
Conversation
31d6199
to
f751edc
Compare
f751edc
to
6eae817
Compare
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.
Lets get this in
class="input input-bordered w-full max-w-md" | ||
value=config_value.get() | ||
on:change=move |ev| { | ||
logging::log!("{:?}", event_target_value(& ev)); |
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.
Can you add the desciption for the log here ?
placeholder="Value" | ||
class="input input-bordered w-full max-w-md pt-3" | ||
on:change=move |ev| { | ||
logging::log!("{:?}", event_target_value(& ev)); |
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.
Here as well
#[prop(default = String::new())] class: String, | ||
#[prop(default = false)] disabled: bool, | ||
) -> impl IntoView { | ||
let (value, set_value) = create_signal(config_value.replace("\\", "")); |
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.
Can we avoid replacing it ?
What if someone actually wants to send this in the input ?
on:change=move |ev| { | ||
logging::log!("{:?}", event_target_value(&ev).parse::<u32>()); | ||
logging::log!("{:?}", event_target_value(& ev).parse::< u32 > ()); |
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.
Can you add the log description here as well ?
if schema.contains_key("enum") { | ||
String::from("ENUM") | ||
} else { | ||
match schema.get("type").unwrap_or(&Value::Null) { |
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 we using unwrap and match both here ?
Possible to change it to :
match schema.get("type") { Some(Value::String(str_)) => str_.to_ascii_uppercase(), _ => String::from("STRING"), }
Moving forward, anything missing can be addressed in a separate PR
Problem
Better form fields
Screen.Recording.2024-07-12.at.12.16.37.PM.mov