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 image support for uikit feature backends #48

Conversation

simlay
Copy link
Contributor

@simlay simlay commented Aug 21, 2022

This builds off #46.

I wanted to add some simple tests so I added Image support for iOS/uikit and put a simple test in with cargo-dinghy. I'm trying to avoid too much of the objc for the future merge conflicts but some I'm sure will happen.

The image used in the test-data is the favicon from simlay.net because I didn't know what else to use.

let image_bytes = include_bytes!("../../test-data/favicon.ico");
self.image = ImageView::new();
self.image.set_image(&Image::with_data(image_bytes));
view.add_subview(&self.image);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't love that this is the way the iOS simulator app looks but it does show that it graphically works.

image

Copy link
Owner

Choose a reason for hiding this comment

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

Ha, I can write up some autolayout hell for this to make it look "nicer" once this is merged. I'd really like to get labels and listview support in, so it'd be cool to get a true "kitchen sink" iOS demo at some point here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. It'd be nice if it could be use similar to the other views of LayoutConstraint::activate. I agree that it should be out of the scope of this PR though.

@ryanmcgrath
Copy link
Owner

This is awesome! At a glance I don't see anything I'm concerned with, but I'll likely take another look tomorrow and then look at merging.

If this also makes it so I don't need to deal with #46 further then I'll close that once this gets merged as well.

src/lib.rs Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@ryanmcgrath
Copy link
Owner

I've gone ahead and reviewed the objc2 PRs with the aim to try and get them merged, after which I think I'll merge this and the iOS fix (and cut a 0.4 most likely, just due to the objc2 changes).

That said, this could probably get merged sooner without too much hassle - just don't want to make too much extra work for @madsmtm existing PRs.

(Your work is, as always, appreciated greatly!)

name = "defaults"
required-features = ["appkit"]
[[example]]
name = "frame_layout"
Copy link
Owner

Choose a reason for hiding this comment

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

I'm back and forth on whether this one should disable autolayout - it's feature flagged for platforms that might not have that, but can still do frame-based layout. I think if autolayout is disabled it may also break the example at the moment, so I can PR this one after this is merged.

@simlay simlay force-pushed the simlay/add-image-support-and-simple-unit-test branch from e8300ce to d80b38c Compare August 21, 2022 23:22
@ryanmcgrath ryanmcgrath merged commit 6b69c1d into ryanmcgrath:trunk Aug 22, 2022
@ryanmcgrath
Copy link
Owner

Merged - thanks again for all of this!

@ryanmcgrath ryanmcgrath mentioned this pull request Aug 22, 2022
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