Skip to content

Commit

Permalink
refactor(core): 💡 Remove PairChild and PairWithChild traits
Browse files Browse the repository at this point in the history
  • Loading branch information
M-Adoo committed Aug 1, 2024
1 parent 82fc5be commit ae63d2f
Show file tree
Hide file tree
Showing 13 changed files with 179 additions and 203 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ Please only add new entries below the [Unreleased](#unreleased---releasedate) he
The `IntoWidget` trait allows for the conversion of any widget to the type `Widget`.
The `IntoChild` trait provides a way to convert a more general widget into a child of `ComposeChild`.

### Fixed

**core**: The generation of a pipe widget from another pipe widget may potentially result in a crash. (#pr, @M-Adoo)

### Changed

- **core**: Simplify the implementation of parent composition with child widgets. (#pr, @M-Adoo)
Expand All @@ -42,6 +46,7 @@ Please only add new entries below the [Unreleased](#unreleased---releasedate) he

- Remove `ChildFrom` and `FromAnother` traits (#pr, @M-Adoo)
- Remove `SingleParent` and `MultiParent` traits. (#pr, @M-Adoo)
- Remove `PairChild` and `PairWithChild` traits. User can use a generic type instead. (#pr, @M-Adoo)
- Allow only the child to be converted to a widget or a type that implements the Into trait. (#pr, @M-Adoo)


Expand Down
8 changes: 0 additions & 8 deletions core/src/builtin_widgets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -947,14 +947,6 @@ impl FatObj<()> {
pub fn with_child<C>(self, child: C, _: &BuildCtx) -> FatObj<C> { self.map(move |_| child) }
}

impl<T: PairWithChild<C>, C> PairWithChild<C> for FatObj<T> {
type Target = Pair<FatObj<T>, C>;

#[inline]
#[track_caller]
fn with_child(self, child: C, _: &BuildCtx) -> Self::Target { Pair::new(self, child) }
}

impl<T> std::ops::Deref for FatObj<T> {
type Target = T;
#[inline]
Expand Down
90 changes: 85 additions & 5 deletions core/src/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use widget_id::RenderQueryable;

use crate::{
builtin_widgets::key::AnyKey,
data_widget::Queryable,
prelude::*,
render_helper::{PureRender, RenderProxy},
ticker::FrameMsg,
Expand Down Expand Up @@ -143,7 +144,6 @@ pub(crate) trait InnerPipe: Pipe {
<Self::Value as IntoIterator>::Item: IntoWidget<M>,
Self: Sized,
{
// fixme: child maybe a pipe, but not replace parent pipe info
let collect_widgets = move |widgets: Self::Value, ctx: &BuildCtx| {
let mut widgets = widgets
.into_iter()
Expand All @@ -163,7 +163,22 @@ pub(crate) trait InnerPipe: Pipe {
let widgets = collect_widgets(m, ctx);
let pipe_node = PipeNode::share_capture(widgets[0].id(), Box::new(info.clone()), ctx);
let ids = widgets.iter().map(|w| w.id()).collect::<Vec<_>>();

set_pos_of_multi(&ids, ctx);

// We need to associate the parent information with the children pipe so that
// when the child pipe is regenerated, it can update the parent pipe information
// accordingly.
{
let arena = &mut ctx.tree.borrow_mut().arena;
for id in &ids[1..] {
if id.assert_get(arena).contain_type::<DynInfo>() {
let info: Box<dyn DynWidgetInfo> = Box::new(info.clone());
id.attach_data(Queryable(info), arena);
};
}
}

info.borrow_mut().widgets = ids;

let c_pipe_node = pipe_node.clone();
Expand Down Expand Up @@ -203,8 +218,6 @@ pub(crate) trait InnerPipe: Pipe {
Self: Sized,
Self::Value: IntoWidget<M>,
{
// fixme: child maybe a pipe, but when it regen, not replace parent pipe info

let root = ctx.tree.borrow().root();
let info = Sc::new(RefCell::new(SingleParentPipeInfo { range: root..=root, multi_pos: 0 }));
let info2 = info.clone();
Expand All @@ -213,8 +226,19 @@ pub(crate) trait InnerPipe: Pipe {
let p = v.into_widget(ctx);

let pipe_node = PipeNode::share_capture(p.id(), Box::new(info.clone()), ctx);
let leaf = p.id().single_leaf(&ctx.tree.borrow().arena);
info.borrow_mut().range = p.id()..=leaf;

info.borrow_mut().range = p.id()..=p.id().single_leaf(&ctx.tree.borrow().arena);
// We need to associate the parent information with the pipe of leaf widget,
// when the leaf pipe is regenerated, it can update the parent pipe information
// accordingly.
{
let arena = &mut ctx.tree.borrow_mut().arena;
if leaf.assert_get(arena).contain_type::<DynInfo>() {
let info: Box<dyn DynWidgetInfo> = Box::new(info.clone());
leaf.attach_data(Queryable(info), arena);
};
}

let c_pipe_node = pipe_node.clone();

Expand Down Expand Up @@ -480,7 +504,6 @@ impl<const M: usize, V: IntoWidget<M> + 'static> IntoWidgetStrict<M> for Box<dyn
}
}

// todo: does impl Pipe<Value = Option<Widget>> is a widget ?
impl<V, S, F, const M: usize> IntoWidget<M> for MapPipe<Option<V>, S, F>
where
V: IntoWidget<M> + 'static,
Expand Down Expand Up @@ -1756,4 +1779,61 @@ mod tests {
wnd.draw_frame();
assert_eq!(&*w2.read(), &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
}

#[test]
fn fix_pipe_in_multi_pipe_not_first() {
reset_test_env!();
let (m_watcher, m_writer) = split_value(0);
let (son_watcher, son_writer) = split_value(0);

let widget = fn_widget! {
@MockMulti {
@ {
pipe!($m_watcher;).map(move |_| [
@ { Void.into_widget(ctx!()) },
@ { pipe!($son_watcher;).map(|_| Void).into_widget(ctx!()) }
])
}
}
};

let mut wnd = TestWindow::new(widget);
wnd.draw_frame();
*son_writer.write() += 1;
wnd.draw_frame();
// If the parent pipe is not correctly collected, it may cause a panic when
// refreshing the child widget of the pipe.
*m_writer.write() += 1;
wnd.draw_frame();
}

#[test]
fn fix_pipe_in_parent_only_pipe_at_end() {
reset_test_env!();
let (m_watcher, m_writer) = split_value(0);
let (son_watcher, son_writer) = split_value(0);

let widget = fn_widget! {
let p = @ {
pipe!($m_watcher;).map(move |_| {
// margin is static, but its child MockBox is a pipe.
let p = pipe!($son_watcher;).map(|_| MockBox { size: Size::zero() });
@$p { margin: EdgeInsets::all(1.) }
})
};
@ $p {
@{ Void }
}
};

let mut wnd = TestWindow::new(widget);
wnd.draw_frame();
*son_writer.write() += 1;
wnd.draw_frame();

// If the parent pipe is not correctly collected, it may cause a panic when
// refreshing the child widget of the pipe.
*m_writer.write() += 1;
wnd.draw_frame();
}
}
37 changes: 0 additions & 37 deletions core/src/widget_children.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@ pub trait WithChild<C, const N: usize, const M: usize> {
fn with_child(self, child: C, ctx: &BuildCtx) -> Self::Target;
}

// todo: remove it.
/// Trait to mark an object that it should compose with its child as a
/// `SinglePair` and the parent and child keep their type.
pub trait PairChild {}

/// Trait mark widget can have one child and also have compose logic for widget
/// and its child.
pub trait ComposeChild: Sized {
Expand Down Expand Up @@ -109,38 +104,6 @@ impl<W, C> Pair<W, C> {
pub fn parent(self) -> W { self.parent }
}

impl<W, C> Pair<FatObj<W>, C> {
/// Replace the host of the FatObj in parent with the child, this is useful
/// when the host of the `FatObj` is useless.
pub fn child_replace_host(self) -> FatObj<C> {
let Self { parent, child } = self;
parent.map(|_| child)
}
}

pub trait PairWithChild<C> {
type Target;
fn with_child(self, child: C, ctx: &BuildCtx) -> Self::Target;
}

impl<W: PairChild, C> PairWithChild<C> for W {
type Target = Pair<W, C>;

#[inline]
#[track_caller]
fn with_child(self, child: C, _: &BuildCtx) -> Self::Target { Pair { parent: self, child } }
}

impl<W, C1: PairChild, C2> PairWithChild<C2> for Pair<W, C1> {
type Target = Pair<W, Pair<C1, C2>>;

#[track_caller]
fn with_child(self, c: C2, ctx: &BuildCtx) -> Self::Target {
let Pair { parent: widget, child } = self;
Pair { parent: widget, child: child.with_child(c, ctx) }
}
}

#[cfg(test)]
mod tests {
use ribir_dev_helper::*;
Expand Down
50 changes: 23 additions & 27 deletions examples/messages/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,43 +73,39 @@ impl Compose for MessageList {
@{ material_svgs::SMS }
@{ Label::new("Messages") }
}
@TabPane {
@ {
fn_widget! {
@VScrollBar {
@Lists {
@{
let message_gen = move |message: Message| {
@Column {
@ListItem {
line_number: 1usize,
@HeadlineText(Label::new(message.nick_name.clone()))
@SupportingText(Label::new(message.content.clone()))
@Leading {
@EdgeWidget::Avatar(@Avatar { @{ message.img.clone() } })
}
@Trailing { @EdgeWidget::Icon(svgs::MORE_HORIZ) }
}
@Divider {}
@TabPane(
fn_widget! {
@VScrollBar {
@Lists {
@{
let message_gen = move |message: Message| {
@Column {
@ListItem {
line_number: 1usize,
@HeadlineText(Label::new(message.nick_name.clone()))
@SupportingText(Label::new(message.content.clone()))
@Leading(
EdgeWidget::Avatar(@Avatar { @{ message.img.clone() } })
)
@Trailing(EdgeWidget::Icon(svgs::MORE_HORIZ.into_widget(ctx!())))
}
};
@Divider {}
}
};

$this.messages.clone().into_iter().map(message_gen)
}
$this.messages.clone().into_iter().map(message_gen)
}
}
}.into()
}
}
}
}.into()
)
}
@Tab {
@TabItem {
@{ material_svgs::ACCOUNT_CIRCLE }
@{ Label::new("Person") }
}
@TabPane {
@{ fn_widget!(@Text { text: "Person" }).into() }
}
@TabPane(fn_widget!(@Text { text: "Person" }).into())
}
}
}
Expand Down
Loading

0 comments on commit ae63d2f

Please sign in to comment.