From 8f12cf15b769bed36e76f11516f23342b1d376d2 Mon Sep 17 00:00:00 2001 From: Pauan Date: Mon, 27 Apr 2020 07:36:48 +0200 Subject: [PATCH] Changing children to use BorrowMut --- src/dom.rs | 36 +++++- src/operations.rs | 302 ++++++++++++++++++++++++---------------------- 2 files changed, 195 insertions(+), 143 deletions(-) diff --git a/src/dom.rs b/src/dom.rs index 635e42a..b1b6062 100644 --- a/src/dom.rs +++ b/src/dom.rs @@ -1,4 +1,5 @@ use std::pin::Pin; +use std::borrow::BorrowMut; use std::convert::AsRef; use std::future::Future; use std::task::{Context, Poll}; @@ -264,6 +265,7 @@ fn create_element_ns(name: &str, namespace: &str) -> A where A: JsCast { } +// TODO should this inline ? fn set_option(element: A, callbacks: &mut Callbacks, value: D, mut f: F) where A: 'static, C: OptionStr, @@ -289,6 +291,7 @@ fn set_option(element: A, callbacks: &mut Callbacks, value: D, mu })); } +// TODO should this inline ? fn set_style(style: &CssStyleDeclaration, name: &A, value: B, important: bool) where A: MultiStr, B: MultiStr { @@ -332,6 +335,7 @@ fn set_style(style: &CssStyleDeclaration, name: &A, value: B, important: b } } +// TODO should this inline ? fn set_style_signal(style: CssStyleDeclaration, callbacks: &mut Callbacks, name: A, value: D, important: bool) where A: MultiStr + 'static, B: MultiStr, @@ -356,6 +360,7 @@ fn set_style_signal(style: CssStyleDeclaration, callbacks: &mut Call // TODO check that the property *actually* was changed ? // TODO maybe use AsRef ? +// TODO should this inline ? fn set_property(element: &A, name: &B, value: C) where A: AsRef, B: MultiStr, C: Into { let element = element.as_ref(); let value = value.into(); @@ -509,6 +514,7 @@ impl DomBuilder where A: AsRef { } impl DomBuilder where A: AsRef { + // TODO should this inline ? fn set_property_signal(&mut self, name: B, value: D) where B: MultiStr + 'static, C: Into, @@ -561,7 +567,7 @@ impl DomBuilder where A: AsRef { // TODO figure out how to make this owned rather than &mut #[inline] - pub fn children<'a, B: IntoIterator>(mut self, children: B) -> Self { + pub fn children<'a, B: BorrowMut, C: IntoIterator>(mut self, children: C) -> Self { self.check_children(); operations::insert_children_iter(self.element.as_ref(), &mut self.callbacks, children); self @@ -575,6 +581,7 @@ impl DomBuilder where A: AsRef { self } + // TODO should this inline ? fn set_text_signal(&mut self, value: C) where B: AsStr, C: Signal + 'static { @@ -670,6 +677,7 @@ impl DomBuilder where A: AsRef { } impl DomBuilder where A: AsRef { + // TODO should this inline ? fn set_attribute_signal(&mut self, name: B, value: E) where B: MultiStr + 'static, C: AsStr, @@ -695,7 +703,6 @@ impl DomBuilder where A: AsRef { }); } - #[inline] pub fn attribute_signal(mut self, name: B, value: E) -> Self where B: MultiStr + 'static, @@ -707,6 +714,8 @@ impl DomBuilder where A: AsRef { self } + + // TODO should this inline ? fn set_attribute_namespace_signal(&mut self, namespace: &str, name: B, value: E) where B: MultiStr + 'static, C: AsStr, @@ -747,6 +756,7 @@ impl DomBuilder where A: AsRef { } + // TODO should this inline ? fn set_class_signal(&mut self, name: B, value: C) where B: MultiStr + 'static, C: Signal + 'static { @@ -794,6 +804,7 @@ impl DomBuilder where A: AsRef { // TODO use OptionStr ? + // TODO should this inline ? fn set_scroll_signal(&mut self, signal: B, mut f: F) where B: Signal> + 'static, F: FnMut(&Element, i32) + 'static { @@ -887,6 +898,7 @@ impl DomBuilder where A: AsRef { } + // TODO should this inline ? fn set_focused_signal(&mut self, value: B) where B: Signal + 'static { @@ -1158,6 +1170,26 @@ mod tests { let _ = a.apply(my_mixin); } + #[test] + fn children_mut() { + let _a: DomBuilder = DomBuilder::new_html("div") + .children(&mut [ + DomBuilder::::new_html("div").into_dom(), + DomBuilder::::new_html("div").into_dom(), + DomBuilder::::new_html("div").into_dom(), + ]); + } + + #[test] + fn children_value() { + let v: Vec = vec![]; + + let _a: DomBuilder = DomBuilder::new_html("div") + .children(v.iter().map(|_| { + DomBuilder::::new_html("div").into_dom() + })); + } + #[test] fn text_signal_types() { let _ = text_signal(always("foo")); diff --git a/src/operations.rs b/src/operations.rs index 80f72c4..3a55073 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -54,14 +54,19 @@ fn for_each_vec(signal: A, mut callback: B) -> CancelableFutureHandle #[inline] -pub(crate) fn insert_children_iter<'a, A: IntoIterator>(element: &Node, callbacks: &mut Callbacks, value: A) { - for dom in value.into_iter() { +pub(crate) fn insert_children_iter, B: IntoIterator>(element: &Node, callbacks: &mut Callbacks, value: B) { + fn insert_children_one(element: &Node, callbacks: &mut Callbacks, dom: &mut Dom) { // TODO can this be made more efficient ? callbacks.after_insert.append(&mut dom.callbacks.after_insert); callbacks.after_remove.append(&mut dom.callbacks.after_remove); bindings::append_child(element, &dom.element); } + + for mut dom in value { + let dom = std::borrow::BorrowMut::borrow_mut(&mut dom); + insert_children_one(element, callbacks, dom); + } } @@ -74,6 +79,7 @@ fn after_insert(is_inserted: bool, callbacks: &mut Callbacks) { } +#[inline] pub(crate) fn insert_child_signal(element: Node, callbacks: &mut Callbacks, signal: A) where A: Signal> + 'static { @@ -82,48 +88,58 @@ pub(crate) fn insert_child_signal(element: Node, callbacks: &mut Callbacks, s child: Option, } - let state = Rc::new(RefCell::new(State { - is_inserted: false, - child: None, - })); + impl State { + fn new() -> Rc> { + Rc::new(RefCell::new(State { + is_inserted: false, + child: None, + })) + } - { - let state = state.clone(); + fn after_insert(state: Rc>, callbacks: &mut Callbacks) { + callbacks.after_insert(move |_| { + let mut state = state.borrow_mut(); - callbacks.after_insert(move |_| { - let mut state = state.borrow_mut(); + if !state.is_inserted { + state.is_inserted = true; - if !state.is_inserted { - state.is_inserted = true; - - if let Some(ref mut child) = state.child { - child.callbacks.trigger_after_insert(); + if let Some(ref mut child) = state.child { + child.callbacks.trigger_after_insert(); + } } + }); + } + + // TODO verify that this will drop `child` + fn after_remove(&mut self, element: &Node, mut child: Option) { + if let Some(old_child) = self.child.take() { + bindings::remove_child(&element, &old_child.element); + + old_child.callbacks.discard(); } - }); + + if let Some(ref mut new_child) = child { + bindings::append_child(&element, &new_child.element); + + after_insert(self.is_inserted, &mut new_child.callbacks); + } + + self.child = child; + } } - // TODO verify that this will drop `child` - callbacks.after_remove(for_each(signal, move |mut child| { + let state = State::new(); + + State::after_insert(state.clone(), callbacks); + + callbacks.after_remove(for_each(signal, move |child| { let mut state = state.borrow_mut(); - - if let Some(old_child) = state.child.take() { - bindings::remove_child(&element, &old_child.element); - - old_child.callbacks.discard(); - } - - if let Some(ref mut new_child) = child { - bindings::append_child(&element, &new_child.element); - - after_insert(state.is_inserted, &mut new_child.callbacks); - } - - state.child = child; + state.after_remove(&element, child); })); } +#[inline] pub(crate) fn insert_children_signal_vec(element: Node, callbacks: &mut Callbacks, signal: A) where A: SignalVec + 'static { @@ -132,130 +148,134 @@ pub(crate) fn insert_children_signal_vec(element: Node, callbacks: &mut Callb children: Vec, } - let state = Rc::new(RefCell::new(State { - is_inserted: false, - children: vec![], - })); + impl State { + fn new() -> Rc> { + Rc::new(RefCell::new(State { + is_inserted: false, + children: vec![], + })) + } - { - let state = state.clone(); + fn after_insert(state: Rc>, callbacks: &mut Callbacks) { + callbacks.after_insert(move |_| { + let mut state = state.borrow_mut(); - callbacks.after_insert(move |_| { - let mut state = state.borrow_mut(); + if !state.is_inserted { + state.is_inserted = true; - if !state.is_inserted { - state.is_inserted = true; + for dom in state.children.iter_mut() { + dom.callbacks.trigger_after_insert(); + } + } + }); + } - for dom in state.children.iter_mut() { - dom.callbacks.trigger_after_insert(); + fn clear(&mut self, element: &Node) { + // TODO is this correct ? + if self.children.len() > 0 { + bindings::remove_all_children(element); + + for dom in self.children.drain(..) { + dom.callbacks.discard(); } } - }); - } + } - fn clear(state: &mut State, element: &Node) { - // TODO is this correct ? - if state.children.len() > 0 { - bindings::remove_all_children(element); + fn insert_at(&self, element: &Node, new_index: usize, child: &Node) { + if let Some(dom) = self.children.get(new_index) { + bindings::insert_child_before(element, child, &dom.element); - for dom in state.children.drain(..) { - dom.callbacks.discard(); + } else { + bindings::append_child(element, child); + } + } + + // TODO verify that this will drop `children` + fn process_change(&mut self, element: &Node, change: VecDiff) { + match change { + VecDiff::Replace { values } => { + self.clear(element); + + self.children = values; + + let is_inserted = self.is_inserted; + + for dom in self.children.iter_mut() { + bindings::append_child(element, &dom.element); + + after_insert(is_inserted, &mut dom.callbacks); + } + }, + + VecDiff::InsertAt { index, mut value } => { + self.insert_at(element, index, &value.element); + + after_insert(self.is_inserted, &mut value.callbacks); + + // TODO figure out a way to move this to the top + self.children.insert(index, value); + }, + + VecDiff::Push { mut value } => { + bindings::append_child(element, &value.element); + + after_insert(self.is_inserted, &mut value.callbacks); + + // TODO figure out a way to move this to the top + self.children.push(value); + }, + + VecDiff::UpdateAt { index, mut value } => { + let dom = &mut self.children[index]; + + bindings::replace_child(element, &value.element, &dom.element); + + after_insert(self.is_inserted, &mut value.callbacks); + + // TODO figure out a way to move this to the top + // TODO test this + ::std::mem::swap(dom, &mut value); + + value.callbacks.discard(); + }, + + VecDiff::Move { old_index, new_index } => { + let value = self.children.remove(old_index); + + self.insert_at(element, new_index, &value.element); + + self.children.insert(new_index, value); + }, + + VecDiff::RemoveAt { index } => { + let dom = self.children.remove(index); + + bindings::remove_child(element, &dom.element); + + dom.callbacks.discard(); + }, + + VecDiff::Pop {} => { + let dom = self.children.pop().unwrap_throw(); + + bindings::remove_child(element, &dom.element); + + dom.callbacks.discard(); + }, + + VecDiff::Clear {} => { + self.clear(element); + }, } } } - fn insert_at(state: &mut State, element: &Node, new_index: usize, child: &Node) { - if let Some(dom) = state.children.get(new_index) { - bindings::insert_child_before(element, child, &dom.element); + let state = State::new(); - } else { - bindings::append_child(element, child); - } - } + State::after_insert(state.clone(), callbacks); - fn process_change(state: &mut State, element: &Node, change: VecDiff) { - match change { - VecDiff::Replace { values } => { - clear(state, element); - - state.children = values; - - let is_inserted = state.is_inserted; - - for dom in state.children.iter_mut() { - bindings::append_child(element, &dom.element); - - after_insert(is_inserted, &mut dom.callbacks); - } - }, - - VecDiff::InsertAt { index, mut value } => { - insert_at(state, element, index, &value.element); - - after_insert(state.is_inserted, &mut value.callbacks); - - // TODO figure out a way to move this to the top - state.children.insert(index, value); - }, - - VecDiff::Push { mut value } => { - bindings::append_child(element, &value.element); - - after_insert(state.is_inserted, &mut value.callbacks); - - // TODO figure out a way to move this to the top - state.children.push(value); - }, - - VecDiff::UpdateAt { index, mut value } => { - let dom = &mut state.children[index]; - - bindings::replace_child(element, &value.element, &dom.element); - - after_insert(state.is_inserted, &mut value.callbacks); - - // TODO figure out a way to move this to the top - // TODO test this - ::std::mem::swap(dom, &mut value); - - value.callbacks.discard(); - }, - - VecDiff::Move { old_index, new_index } => { - let value = state.children.remove(old_index); - - insert_at(state, element, new_index, &value.element); - - state.children.insert(new_index, value); - }, - - VecDiff::RemoveAt { index } => { - let dom = state.children.remove(index); - - bindings::remove_child(element, &dom.element); - - dom.callbacks.discard(); - }, - - VecDiff::Pop {} => { - let dom = state.children.pop().unwrap_throw(); - - bindings::remove_child(element, &dom.element); - - dom.callbacks.discard(); - }, - - VecDiff::Clear {} => { - clear(state, element); - }, - } - } - - // TODO maybe move this into the after_insert callback and remove State - // TODO verify that this will drop `children` callbacks.after_remove(for_each_vec(signal, move |change| { let mut state = state.borrow_mut(); - - process_change(&mut state, &element, change); + state.process_change(&element, change); })); }