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 support for localized image #183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cyrildurand
Copy link

fix #182

need further test

Let me know what you think about it

@tomasmcguinness
Copy link
Owner

Thanks for this - sorry I've not looked it before now.

I'll review this week and let you know!

@tomasmcguinness
Copy link
Owner

Sorry - I've not gotten around to this yet!

@tomasmcguinness
Copy link
Owner

It didn't initially compile - I think the Culture and LanguageCode properties got mixed up?

The filename doesn't include the lproj extension that was shown in your screenshot. I guess that should be there?

The API for adding the images doesn't allow you to specify the language code for a particular image. We currently allow images to be added like this:

request.Images.Add(PassbookImage.Icon, System.IO.File.ReadAllBytes(Server.MapPath("~/Icons/icon.png")));

We probably need a new way to add images, perhaps a method like:

request.AddImage(PassbookImage.Icon, System.IO.File.ReadAllBytes(Server.MapPath("~/Icons/icon.png")));
request.AddImage(PassbookImage.Icon, "en", System.IO.File.ReadAllBytes(Server.MapPath("~/Icons/icon.png")));

What do you think?

@cyrildurand
Copy link
Author

I just viewed your comment

What you mean by lproj ? there is no lproj on my commit.

In my commit, I change PassbookImage from enum to struct that contains the image type and the optional language code.
This way we ensure compatibility with existing source code.

I use a fork and it compiles successfully. I have other changed and am willing to contribute to this project.

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.

Add support for localized images
2 participants