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 round scroll bar option #96

Merged
merged 5 commits into from
Aug 26, 2023
Merged

Conversation

jrmoulton
Copy link
Contributor

@jrmoulton jrmoulton commented Aug 24, 2023

This adds the option for a scroll bar to be rounded and makes that the default on macOS

Update: this now also

  1. changes the View style methods to take a base style in function argument
  2. Moves the scroll bar styles to the View style and makes them apply to all children elements until override like font size

@dzhou121
Copy link
Contributor

Maybe let's create a ScrollBarStyle which has bar color, bar size, and radius etc.

@jrmoulton
Copy link
Contributor Author

jrmoulton commented Aug 24, 2023

I think this is good. I don't love having to import ScrollBarStyle and create a new instance of it to add it to the function. Something SwiftUI-esque like

.scroll_bar_style( || .color(Color::GREEN) )

would be nice but I don't know of any way to do this so I think this is good enough

@jrmoulton
Copy link
Contributor Author

I guess this is possible...

    pub fn scroll_bar_style(
        self,
        style: impl Fn(ScrollBarStyle) -> ScrollBarStyle + 'static,
    ) -> Self {
        let id = self.id;
        create_effect(move |_| {
            let style = style(ScrollBarStyle::BASE);
            id.update_state(UpdateState::Style(style), false);
        });

        self
    }

and then

        .scroll_bar_style(|base| base.color(custom_scroll_bar_color()))

@jrmoulton
Copy link
Contributor Author

and then

        .scroll_bar_style(|base| base.color(custom_scroll_bar_color()))

I actually really quite like this for styles. Would actually be a nice way to do View styles as well

@jrmoulton
Copy link
Contributor Author

jrmoulton commented Aug 24, 2023

#97 affects how this pr is viewed

@dzhou121
Copy link
Contributor

I agree. This is very nice. We could do the same for view style.

@dzhou121
Copy link
Contributor

On the scroll bar style, I feel like we need to put it in the view style and let it affect child views like the way font size does. So that you can only specify it once at the root. Still you have the ability to customise it at a child view.

@jrmoulton
Copy link
Contributor Author

On the scroll bar style, I feel like we need to put it in the view style and let it affect child views like the way font size does. So that you can only specify it once at the root. Still you have the ability to customise it at a child view.

Ok. Looks like that's done by adding a current / saved value to the paint and a layout contexts?

@dzhou121
Copy link
Contributor

On the scroll bar style, I feel like we need to put it in the view style and let it affect child views like the way font size does. So that you can only specify it once at the root. Still you have the ability to customise it at a child view.

Ok. Looks like that's done by adding a current / saved value to the paint and a layout contexts?

Yes.

@jrmoulton
Copy link
Contributor Author

And when adding these to the view style should we keep them together (in a ScrollBarStyle) struct or separate them? Together seems better for the implementation but it'll be ugly to use

@dzhou121
Copy link
Contributor

Feels like we should seperate them since all other styles are "flat".

@jrmoulton
Copy link
Contributor Author

Minimal testing but does seem to work with applying to child views and overrides also work.This also switches the style method to take a Style and the BASE style is passed in

@dzhou121 dzhou121 merged commit 00a1214 into lapce:main Aug 26, 2023
2 of 5 checks passed
@jrmoulton jrmoulton deleted the rounded-scroll-bar branch September 18, 2024 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants