fix: add scrollable wrapper to device views#586
Open
debarkak wants to merge 2 commits into
Open
Conversation
…t constraints to shrink
Author
|
this PR is related to #241 |
kavishdevar
requested changes
May 11, 2026
Addresses maintainer feedback from PR kavishdevar#586 review: - Move scrollable() into airpods_view() and nothing_view() so each view owns its own scroll behavior - Revert window.rs to call views directly without external wrapping - Fix indentation in the Nothing branch of window.rs - Views now fill available space with height(Length::Fill) and scroll internally when content exceeds viewport
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what was the issue
so when you resize the window to be smaller, the content in the device view just gets clipped and theres no way to scroll to see the rest. this is because the outer container in
airpods_viewandnothing_viewhad.height(Length::Fill)which tells iced to just stretch the container to fill whatever space is available and clip anything that doesnt fit, instead of actually scrolling.update/information
as @kavishdevar wrote a review:
as for this review i have made these changes:
scrollable()insideairpods_viewandnothing_viewso the scroll is part of the view itself.height(Length::Fill)window.rsto callairpods_view/nothing_viewdirectly without any scrollable or container wrappingscrollableimport toairpods.rsandnothing.rsand my reply:
testing (new)
compiled and tested on fedora linux 44 on kde plasma 6.6.4 with airpods 4 anc
Screencast_20260512_134043.webm
what i changed (old)
.height(Length::Fill)to.height(Length::Shrink)inairpods.rs(line 523) andnothing.rs(line 178) so the content sizes to its natural height instead of forcing itself to fill the paneairpods_viewandnothing_viewcalls inwindow.rswithcontainer(scrollable(view).height(Length::Fill))so the views actually scroll when the window is too small to fit everythingthe
scrollablewidget was already imported inwindow.rsso no new imports neededtesting (old)
compiled and tested on fedora linux 43 on gnome with airpods 4 anc, resizing the window small now scrolls properly instead of clipping
the issue was mentioned by kavish on discord, i didn't even notice it until he mentioned it lol
screencast (testing)
Screencast.From.2026-05-07.13-46-56.mp4