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

Multiple changes I had to make for Ebou #89

Merged

Conversation

terhechte
Copy link
Contributor

This PR contains a couple of changes:

Segmented Control

This adds a segmented control. In order to know which index was selected (crucial to do something useful) I had to extend set_action to also receive an object (*const Object) which can then be inspected:

       let mut control = SegmentedControl::new(images, cacao::appkit::segmentedcontrol::TrackingMode::SelectOne);
        control.set_action(|index| {
            println!("Selected Index {index}");
        });

This is a bit of a pervasive change that goes through the whole codebase

Formatting

I'm using the default Rust Analysers auto formatting rules in all my code and somehow my settings are different than the one used in this codebase, so it added a lot of commas in places. I'm not sure if this is a problem for you or not.

Various

  • Support for SegmentedControl in Toolbar
  • Added more SFSymbols
  • Support loading images from URLs

@terhechte
Copy link
Contributor Author

I can look into the conflicts once I know whether you'd be willing to merge this PR :)

@ryanmcgrath
Copy link
Owner

I'd say I'm probably down to merge this, because you're actually using it in a production application and discovering things that are necessary. :)

The aspect of it being a slightly invasive change is fine since this repo is prepping for a 0.4 anyway.

@terhechte
Copy link
Contributor Author

Awesome! I've fixed the merge conflicts 😄

@ryanmcgrath ryanmcgrath merged commit e6696ea into ryanmcgrath:trunk Aug 1, 2023
@ryanmcgrath
Copy link
Owner

Thanks!

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