Conversation
xStrom
added
masonry
jaredoconnell
left a comment
jaredoconnell
left a comment
There was a problem hiding this comment.
I haven't reviewed it deeply enough to approve it, but from my once-over and testing, the behavior looks pretty good quality already. The only oddity due to how it's implemented is that the button you press is the one that stays active, despite the number changing. The pro of this behavior is you can hit enter again to skip ahead the same amount again, until it stops shifting because it gets to the end of the range. But this behavior isn't consistent, and is therefore less intuitive.
|
Yes, I also noticed that focus behavior during my testing. The problem is that Masonry currently only allows changing the focus target from the widget's own event handling. So the host |
| /// Collections of context methods required to sync buttons. | ||
| trait SyncCtx { | ||
| fn children_changed(&mut self); | ||
| fn set_stashed(&mut self, child: &mut WidgetPod<impl Widget + ?Sized>, stashed: bool); | ||
| fn remove_child(&mut self, child: WidgetPod<impl Widget + ?Sized>); | ||
| fn mutate_child_later<W: Widget + FromDynWidget + ?Sized>( | ||
| &mut self, | ||
| child: &mut WidgetPod<W>, | ||
| f: impl FnOnce(WidgetMut<'_, W>) + Send + 'static, | ||
| ); | ||
| } |
There was a problem hiding this comment.
I'm really cautious of adding more traits to Masonry, though this one is at least private.
An alternative I'd recommend is using MutateCtx::update_mut instead and have sync_buttons take &mut UpdateCtx. You'd have to add a similar method to ActionCtx.
Currently MutateCtx::update_mut is private, and I think the reason is because making public would mean we'd commit to having MutateCtx (and ActionCtx, and probably EventCtx) be a superset of UpdateCtx, basically locking in "you can do everything during the mutate pass".
This is probably worth a Zulip discussion, and in the meantime this solution is fine.
There was a problem hiding this comment.
Yeah I only added the trait out of necessity, but the alternative you mention could work as well. As would having official traits for the context methods. It's a bigger issue than this PR here though and not high priority, so I'll open a Zulip thread for it.
3 participants
The
Paginationwidget is technically new ground for Masonry, as it composes multiple other widgets (Buttonin this case) and then captures and translates their actions (ButtonPress) into its own actions (PageChanged). This builds on top of the new action pass added in #1677.In addition to being able to set the total page count and currently active page index, it's also possible to configure how many pagination buttons should be shown in total and at each of the extremes. Changes to any of these values or any user input that triggers a button press will then execute
sync_buttonswhich is a mini-reactivity engine inside ofPaginationthat makes sure that there is a correct number of childButtonwidgets and that they all have correct page numbers on them. These child widgets are lazily created and when no longer needed, stashing is preferred. So in the case where there are 10 buttons max but only 3 pages exist, only 3 button widgets will be created. Then, let's say a search filter is modified and now there are suddenly 30 pages. The remaining 7 buttons will be created, reaching the total max of 10 buttons. If the filter is modified again and there's only 1 page of results now, 9 of the buttons will be stashed. Only when the maximum number of buttons is reduced to say 5 willPaginationremove the excess buttons.Styling is intentionally postponed until the properties system gains more features, e.g. #1691. Until then, it's just a concated list of buttons with their default style.
You can try it out in the gallery via
cargo run --example gallery.