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

fix input.value set #35

Merged
merged 6 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

57 changes: 57 additions & 0 deletions demo_respo/src/inner_text.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//! a demo for switching inner-text and children, which might cause a bug in respo

tiye marked this conversation as resolved.
Show resolved Hide resolved
use std::fmt::Debug;

use respo::{button, css::RespoStyle, div, span, ui::ui_button, util, DispatchFn, RespoElement, RespoEvent};
use respo_state_derive::RespoState;
use serde::{Deserialize, Serialize};

use respo::states_tree::{RespoState, RespoStatesTree};

use super::store::ActionOp;

#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, RespoState)]
struct InnerTextState {
inner_text: bool,
}

pub fn comp_inner_text(states: &RespoStatesTree) -> Result<RespoElement<ActionOp>, String> {
let cursor = states.path();

let state = states.cast_branch::<InnerTextState>();

let on_inc = {
let cursor = cursor.to_owned();
let state = state.to_owned();
move |e, dispatch: DispatchFn<_>| -> Result<(), String> {
util::log!("click {:?}", e);
if let RespoEvent::Click { original_event, .. } = e {
original_event.prevent_default();
}

dispatch.run(ActionOp::Increment)?;
dispatch.run_state(
&cursor,
InnerTextState {
inner_text: !state.inner_text,
},
)?;
Ok(())
}
};

Ok(
div().elements([
div().elements([button()
.class(ui_button())
.inner_text("Switch inner text")
.style(RespoStyle::default().margin(4.))
.on_click(on_inc)]),
div().elements([if state.inner_text {
div().inner_text("inner text")
} else {
div().elements([span().inner_text("child 1"), span().inner_text("child 2")])
}]),
]),
)
}
9 changes: 8 additions & 1 deletion demo_respo/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
extern crate console_error_panic_hook;

mod counter;
mod inner_text;
mod panel;
mod plugins;
mod store;
Expand All @@ -11,7 +12,8 @@ use std::cell::{Ref, RefCell};
use std::panic;
use std::rc::Rc;

use respo::RespoAction;
use inner_text::comp_inner_text;
use respo::{space, RespoAction};
use web_sys::Node;

use respo::ui::ui_global;
Expand Down Expand Up @@ -65,9 +67,14 @@ impl RespoApp for App {
.style(RespoStyle::default().padding(12.0))
.children([
comp_counter(&states.pick("counter"), store.counted)?.to_node(),
space(None, Some(80)).to_node(),
comp_panel(&states.pick("panel"))?,
comp_todolist(&states.pick("todolist"), &store.tasks)?.to_node(),
space(None, Some(80)).to_node(),
comp_plugins_demo(&states.pick("plugins-demo"))?.to_node(),
space(None, Some(80)).to_node(),
comp_inner_text(&states.pick("inner-text"))?.to_node(),
space(None, Some(80)).to_node(),
])
.to_node(),
)
Expand Down
2 changes: 1 addition & 1 deletion respo/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "respo"
version = "0.1.9"
version = "0.1.13"
edition = "2021"
description = "a tiny virtual DOM library migrated from ClojureScript"
license = "Apache-2.0"
Expand Down
6 changes: 4 additions & 2 deletions respo/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ pub trait RespoApp {
DispatchFn::new(dispatch_action),
Self::get_loop_delay(),
)
.expect("rendering node");
.unwrap_or_else(|e| {
util::error_log!("render loop error: {:?}", e);
});

Ok(())
}
Expand Down Expand Up @@ -123,7 +125,7 @@ pub trait RespoApp {
*store.borrow_mut() = s;
}
Err(e) => {
util::log!("error: {:?}", e);
util::error_log!("error: {:?}", e);
}
},
_ => {
Expand Down
26 changes: 24 additions & 2 deletions respo/src/app/diff.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::cell::RefCell;
use std::collections::{HashMap, HashSet};
use std::fmt::Debug;
use std::rc::Rc;
Expand Down Expand Up @@ -109,7 +110,8 @@ where
});
collect_effects_outside_in_as(new_tree, coord, dom_path, RespoEffectType::Mounted, changes)?;
} else {
diff_attrs(attrs, old_attrs, coord, dom_path, changes);
let reset_inner = RefCell::new(false);
diff_attrs(attrs, old_attrs, coord, dom_path, changes, &reset_inner);
diff_style(
&HashMap::from_iter(style.0.to_owned()),
&HashMap::from_iter(old_style.0.to_owned()),
Expand All @@ -119,7 +121,12 @@ where
);

diff_event(event, old_event, coord, dom_path, changes);
diff_children(children, old_children, coord, dom_path, changes)?;
if *reset_inner.borrow() {
// children is empty after innerHTML or innerText changed
diff_children(children, &[], coord, dom_path, changes)?;
} else {
diff_children(children, old_children, coord, dom_path, changes)?;
}
}
}
(RespoNode::Referenced(new_cell), RespoNode::Referenced(old_cell)) => {
Expand Down Expand Up @@ -147,6 +154,7 @@ fn diff_attrs<T>(
coord: &[RespoCoord],
dom_path: &[u32],
changes: &mut Vec<DomChange<T>>,
reset_inner: &RefCell<bool>,
) where
T: Debug + Clone,
{
Expand All @@ -156,15 +164,24 @@ fn diff_attrs<T>(
if old_attrs.contains_key(key) {
if &old_attrs[key] != value {
added.insert(key.to_owned(), value.to_owned());
if inner_changed(key) {
*reset_inner.borrow_mut() = true;
}
}
} else {
added.insert(key.to_owned(), value.to_owned());
if inner_changed(key) {
*reset_inner.borrow_mut() = true;
}
}
}

for key in old_attrs.keys() {
if !new_attrs.contains_key(key) {
removed.insert(key.to_owned());
if inner_changed(key) {
*reset_inner.borrow_mut() = true;
}
}
}

Expand All @@ -178,6 +195,11 @@ fn diff_attrs<T>(
}
}

/// changed innerHTML or innerText, which resets children values
fn inner_changed(key: &Rc<str>) -> bool {
key == &"innerHTML".into() || key == &"innerText".into() || key == &"inner-text".into()
}

fn diff_style<T>(
new_style: &HashMap<Rc<str>, String>,
old_style: &HashMap<Rc<str>, String>,
Expand Down
109 changes: 85 additions & 24 deletions respo/src/app/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use web_sys::console::warn_1;
use crate::node::{RespoComponent, RespoEffectType, RespoEvent, RespoEventMark, RespoEventMarkFn, RespoNode};

use super::renderer::load_coord_target_tree;
use super::util;
use crate::node::dom_change::{ChildDomOp, DomChange, RespoCoord};

use crate::app::renderer::build_dom_tree;
Expand Down Expand Up @@ -59,7 +60,7 @@ where
}
}
} else {
crate::util::log!("expected component for effects, got: {}", target_tree);
crate::util::warn_log!("expected component for effects, got: {}", target_tree);
}
}
}
Expand Down Expand Up @@ -198,7 +199,10 @@ where
.expect("get node")
.children()
.item(*idx)
.ok_or_else(|| format!("child to remove not found at {}", &idx))?;
.ok_or_else(|| {
util::warn_log!("child not found at {:?}", coord);
format!("child to remove not found at {}", &idx)
})?;
target.remove_child(&child).expect("child removed");
}
ChildDomOp::InsertAfter(idx, k, node) => {
Expand Down Expand Up @@ -243,7 +247,7 @@ where
}
}
} else {
crate::util::log!("expected component for effects, got: {}", target_tree);
crate::util::warn_log!("expected component for effects, got: {}", target_tree);
}
}
}
Expand Down Expand Up @@ -272,7 +276,7 @@ where
}
}
} else {
crate::util::log!("expected component for effects, got: {}", target_tree);
crate::util::warn_log!("expected component for effects, got: {}", target_tree);
}
}
}
Expand Down Expand Up @@ -386,11 +390,25 @@ pub fn attach_event(element: &Element, key: &str, coord: &[RespoCoord], handle_e
.run(RespoEventMark::new("change", &coord, wrap_event))
.expect("handle change event");
}) as Box<dyn FnMut(InputEvent)>);
element
.dyn_ref::<HtmlInputElement>()
.expect("convert to html input element")
.set_onchange(Some(handler.as_ref().unchecked_ref()));
handler.forget();
match element.tag_name().as_str() {
"INPUT" => {
element
.dyn_ref::<HtmlInputElement>()
.expect("convert to html input element")
.set_onchange(Some(handler.as_ref().unchecked_ref()));
handler.forget();
}
"TEXTAREA" => {
element
.dyn_ref::<HtmlTextAreaElement>()
.expect("convert to html input element")
.set_onchange(Some(handler.as_ref().unchecked_ref()));
handler.forget();
}
_ => {
util::warn_log!("not handled change event for element: {}", element.tag_name());
}
}
}
"keydown" => {
let handler = Closure::wrap(Box::new(move |e: KeyboardEvent| {
Expand All @@ -409,11 +427,26 @@ pub fn attach_event(element: &Element, key: &str, coord: &[RespoCoord], handle_e
.run(RespoEventMark::new("keydown", &coord, wrap_event))
.expect("handle keydown event");
}) as Box<dyn FnMut(KeyboardEvent)>);
element
.dyn_ref::<HtmlInputElement>()
.expect("convert to html input element")
.set_onkeydown(Some(handler.as_ref().unchecked_ref()));
handler.forget();

match element.tag_name().as_str() {
"INPUT" => {
element
.dyn_ref::<HtmlInputElement>()
.expect("convert to html input element")
.set_onkeydown(Some(handler.as_ref().unchecked_ref()));
handler.forget();
}
"TEXTAREA" => {
element
.dyn_ref::<HtmlTextAreaElement>()
.expect("convert to html input element")
.set_onkeydown(Some(handler.as_ref().unchecked_ref()));
handler.forget();
}
_ => {
util::warn_log!("not handled keydown event for element: {}", element.tag_name());
}
}
}
"keyup" => {
let handler = Closure::wrap(Box::new(move |e: KeyboardEvent| {
Expand All @@ -431,11 +464,25 @@ pub fn attach_event(element: &Element, key: &str, coord: &[RespoCoord], handle_e
.run(RespoEventMark::new("keyup", &coord, wrap_event))
.expect("handle keyup event");
}) as Box<dyn FnMut(KeyboardEvent)>);
element
.dyn_ref::<HtmlInputElement>()
.expect("convert to html input element")
.set_onkeyup(Some(handler.as_ref().unchecked_ref()));
handler.forget();
match element.tag_name().as_str() {
"INPUT" => {
element
.dyn_ref::<HtmlInputElement>()
.expect("convert to html input element")
.set_onkeyup(Some(handler.as_ref().unchecked_ref()));
handler.forget();
}
"TEXTAREA" => {
element
.dyn_ref::<HtmlTextAreaElement>()
.expect("convert to html input element")
.set_onkeyup(Some(handler.as_ref().unchecked_ref()));
handler.forget();
}
_ => {
util::warn_log!("not handled keyup event for element: {}", element.tag_name());
}
}
}
"keypress" => {
let handler = Closure::wrap(Box::new(move |e: KeyboardEvent| {
Expand All @@ -453,11 +500,25 @@ pub fn attach_event(element: &Element, key: &str, coord: &[RespoCoord], handle_e
.run(RespoEventMark::new("keypress", &coord, wrap_event))
.expect("handle keypress event");
}) as Box<dyn FnMut(KeyboardEvent)>);
element
.dyn_ref::<HtmlInputElement>()
.expect("convert to html input element")
.set_onkeypress(Some(handler.as_ref().unchecked_ref()));
handler.forget();
match element.tag_name().as_str() {
"INPUT" => {
element
.dyn_ref::<HtmlInputElement>()
.expect("convert to html input element")
.set_onkeypress(Some(handler.as_ref().unchecked_ref()));
handler.forget();
}
"TEXTAREA" => {
element
.dyn_ref::<HtmlTextAreaElement>()
.expect("convert to html input element")
.set_onkeypress(Some(handler.as_ref().unchecked_ref()));
handler.forget();
}
_ => {
util::warn_log!("not handled keypress event for element: {}", element.tag_name());
}
}
}
"focus" => {
let handler = Closure::wrap(Box::new(move |e: FocusEvent| {
Expand Down
Loading
Loading