-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Minor Scrollable Improvements #1878
Conversation
Good catch on this! |
What is the purpose of changing the mouse coordinates like this? Line 467 in fcb1b45
This will cause strange behaviors when trying scroll with the mouse outside of the window bounds with nested scrollables. use iced::widget::scrollable::{Properties, Viewport};
use iced::widget::{Container, Scrollable};
use iced::{
executor, widget, Application, Background, Color, Command, Element, Length, Settings, Size,
Theme,
};
pub fn main() -> iced::Result {
HelloWorld::run(Settings::default())
}
#[derive(Debug, Clone, Copy)]
pub enum Message {
Scrolled(Viewport),
}
struct HelloWorld {}
impl Application for HelloWorld {
type Executor = executor::Default;
type Message = Message;
type Theme = Theme;
type Flags = ();
fn new(_flags: Self::Flags) -> (Self, Command<Self::Message>) {
(HelloWorld {}, Command::none())
}
fn title(&self) -> String {
String::from("Hello World!")
}
fn update(&mut self, _message: Message) -> Command<Self::Message> {
Command::none()
}
fn view(&self) -> Element<Message> {
Scrollable::new(
Scrollable::new(
Container::new("Foo")
.style(|theme: &iced::Theme| widget::container::Appearance {
text_color: None,
background: Some(Background::Color(Color::new(1.0, 0.2, 0.3, 1.0))),
border_radius: Default::default(),
border_width: 0.0,
border_color: Default::default(),
})
.width(Length::Fixed(5000.0))
.height(Length::Fixed(5000.0)),
)
.on_scroll(Message::Scrolled)
.width(Length::Fill)
.height(Length::Fill)
.vertical_scroll(Properties::default())
.horizontal_scroll(Properties::default()),
)
.into()
}
fn theme(&self) -> Theme {
Theme::default()
}
} |
I also messed with making vertical scroll optional when I was adding horizontal support, but couldn't see the usefulness of a situation where a enum ScrollDirection {
#[default]
Vertical,
Horizontal,
Both,
} which might be a better representation of how it's actually being used? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bungoboingo I agree. A scrollable
that does not scroll does not make sense. Use another widget instead.
Also, we definitely do not want scrollable
to disable scrolling on both axes by default, which is what this PR is currently doing.
Co-Authored-By: Austin M. Reppert <austinmreppert@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thanks 🙇
I have just renamed ScrollbarProperties
to simply Direction
and simplified some stuff here and there.
We can merge this! 🚢
This PR provides minor improvements for the Scrollable widget.