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

Add use_window_resize_status hook #47

Closed

Conversation

bcorey
Copy link

@bcorey bcorey commented Jun 16, 2024

Introduces a wrapper over the use_window_size hook that tells a component whether or not the window has been resized since the last update. This will help devs prevent expensive component size computations from running when not needed.

@marc2332 marc2332 added the enhancement New feature or request label Jun 16, 2024
Copy link
Collaborator

@marc2332 marc2332 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From how I see it, a new hook is unnecessary. Instead, I would modify the existing use_window_size hook so it checks whether the size is really different or not:

pub fn use_window_size() -> ReadOnlySignal<WindowSize> {
    let mut window_size = use_signal(get_window_size);
    // Initialize the handler
    let tx = use_coroutine(|mut rx: UnboundedReceiver<WindowSize>| async move {
        while let Some(data) = rx.next().await {
            if *window_size.peek() != data {
                window_size.set(data);
            }
        }
    });
    listen(tx);
    use_hook(|| ReadOnlySignal::new(window_size))
}

But even after these changes I don't think it's going to make a different, if a resize event happened then it means the size really changed, there is not a single case where the signal is going to be updated with the exact same size

@marc2332
Copy link
Collaborator

Can you maybe share a bit of context on how you are hitting this expensive computation?

@bcorey
Copy link
Author

bcorey commented Jun 16, 2024

Hi Marc, thanks for taking a look so quickly. I think when it comes to simple cases where this hook is the only hook that will cause a component update you're right - there's no difference in the number of refreshes. but what if the content of a different hook in the component is changed? I'd like to choose what work to do based on which hook caused the refresh. Let me know if there's a more idiomatic way of doing this, here is additional context:

I am arranging a grid of draggable panels within a DragArea with the grid layout based on window size and I'd like to only run the sizing calculations when the window size changes instead of on every single component update of the parent DragArea. This component experiences many window-size-unrelated updates since itself and all of its direct child DragTargets and Draggables already have to rerender with every pointermove event while dragging. Here is a loose overview of the main logic:

    // when the window is resized:
    // DragArea: get dragArea domrect size via web_sys
    // DragArea: divide dragarea size by desired column count
    // DragArea: divide dragarea size by desired row count
    // DragArea: if resulting row or column size is too small, alter desired row or column counts and repeat
    // 
    // DragArea: send new column and row data to targets (causes refresh of all targets and draggables)
    // DragTarget: targets resize to new data
    // Draggable: docked draggables resize to new data

    // when pointerdown on a draggable:
    // DragArea: start recording pointerpos (refreshes DragArea on every pointermove)
    // Draggable: move active draggable to pointerpos
    // Draggable: each docked draggable checks if pointerpos is within bounds
    //       clears space by moving to active draggable source target if so
    //       if previously moved to clear space, moves to original target if not
    // DragTarget: check if pointer is within rect
    //      set global snap data to rect if so (causes dragarea refresh)

here's what I am envisioning for the DragArea hooks:

#[component]
pub fn DragArea(children: Element) -> Element {
    let global_drag_info = use_context_provider(|| Signal::new(GlobalDragState::new())); // causes refresh on pointermove during a drag
    let drag_area_grid = use_context_provider(|| Signal::new(GridLayout::new()));

    let window_resize_status = use_window_resize_status(); // causes refresh on change
    if let Resized(new_size) = window_resize_status { // don't update all the grid stuff just due to a pointermove while dragging
        drag_area_grid.write().set_new_size(new_size); // will cause an additional refresh of this and all children with the new grid data
    }
    rsx!{...}
 }

@DogeDark
Copy link
Member

DogeDark commented Jun 16, 2024

I would suggest looking at use_effect or use_memo. They only re-run when the signals used in them change. e.g.

#[component]
pub fn DragArea(children: Element) -> Element {
    let global_drag_info = ...;
    let drag_area_gird = ...;

    let window_size = use_window_size();
    use_effect(move || {
        // This subscribes this effect to the signal returned by `use_window_size`
        // and will only re-run when it's value changes.
        let size = window_size();

        // Do your computational code
        drag_area_grid.write().set_new_size(size);
    });

    rsx!{...}
}

See use_effect & use_memo

@bcorey
Copy link
Author

bcorey commented Jun 20, 2024

Thanks for pointing out these two hooks - they look promising but I'm still encountering an issue this PR solves: signals written to from inside the use_memo or use_hook closure seem to be cloned or do not update the signal value stored in the component (I have already checked and the code is indeed running when expected, just not storing the new values in the signal). Is there an easy way around this? Here are how the two approaches look for me in the current iteration:

#[component]
fn Draggable(...) -> Element {
    let id = use_signal(|| uuid::Uuid::new_v4().to_string());
    let mut local_drag_info =
        use_context_provider(|| Signal::new(LocalDragState::new(variant, id())));
    let global_drag_info = use_context::<Signal<GlobalDragState>>();


    // let window_size = use_window_size();
    // use_memo(move || {
    //     let _ctx = window_size();
    //     // local_drag_info is undesirably cloned/does not mutate the signal stored by the component
    //     local_drag_info.write().resize_snapped();
    // });

    let window_size_info = use_window_resize_status();
    // local_drag_info mutates the signal stored by the component as desired
    if let WindowSizeWithStatus::Resized(_new_size) = window_size_info {
        local_drag_info.write().resize_snapped();
    }

    [...]

    rsx! {...}
}

@DogeDark
Copy link
Member

It is normal for signals themselves to be cloned but the value stored inside may or may not be depending on how it's used. In your case, the inner value shouldn't be cloned at all since .write() returns a mutable reference. As for the signal not updating, what method are you using to check it?

@bcorey
Copy link
Author

bcorey commented Jun 30, 2024

I've investigated further and filed DioxusLabs/dioxus/#2565 to account for the issue I'm having with use_memo

@bcorey
Copy link
Author

bcorey commented Jul 5, 2024

closing now that I have the use_memo approach working. thanks for the help!

@bcorey bcorey closed this Jul 5, 2024
@bcorey bcorey deleted the bcorey/use_window_resize_status branch July 5, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants