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

[TIMOB-25619] Android: Allow non-file TiBlob to be previewed #9771

Merged
merged 1 commit into from Feb 28, 2018

Conversation

garymathews
Copy link
Contributor

  • Allow non-file TiBlob to be previewed
TEST CASE
var win = Ti.UI.createWindow({ backgroundColor: 'green' });

win.addEventListener('postlayout', function () {
    win.toImage(function (blob) {
        if (!blob) return;

        win.backgroundColor = 'red';

        Ti.Media.previewImage({
            image: blob,
            success: function (e) {
                console.log('success, displaying preview');
            },
            error: function (e) {
                console.error('failed to display preview');
            },
        });
    });
});

win.open();

JIRA Ticket

@build
Copy link
Contributor

build commented Jan 26, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

CR: Pass

Copy link
Contributor

@ypbnv ypbnv left a comment

Choose a reason for hiding this comment

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

CR: Pass

Uri imageUri = TiFileProvider.createUriFrom(f.getNativeFile());
Uri imageUri = null;

if (image.getNativePath() == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

may want to consider refactoring this handling with the equivalent code in EmailDialogProxy. Basically in both instances you're trying to get a TiBlob converted to a File.

Copy link
Contributor

Choose a reason for hiding this comment

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

For everyone's info, the way EmailDialogProxy is saving blobs to a temp file needs to be refactored too. It always saves blobs to the same filename. So, the last blob saved gets overwritten by the next blob saved to file.
https://jira.appcelerator.org/browse/TIMOB-25568

I recommend that we refactor our handling when addressing TIMOB-25568.

@@ -38,6 +38,37 @@ describe('Titanium.Media', function () {
should(Ti.Media.previewImage).be.a.Function;
});

it.android('previewImage of non-file blob', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

my bad, I moved this test into the suite so there's a conflict now. You can move this to a ti.media.addontest.js or copy the ti/media.test front he suite and add this (or uncomment the variant of this I wrote using takeScreenshot which also exposed the issue).

Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

FR Passed Was able to see a a preview of non-file TiBlob

Test Steps:

  • Created a titanium project with the build form this PR
  • Added the above code in to the project
  • Ran the project
  • Was able to view the blob without any runtime errors (which were seen on older versions of the SDK
  • Also saw the following in the console

[INFO] : success, displaying preview

Test Environment
APPC Studio: 5.0.0.201711280737
APPC CLI: 7.0.1
Device: Nexus 6p (8.1.0)
Operating System Name: Mac OS High Sierra
Operating System Version: 10.13
Node.js Version: 8.9.1

@garymathews
Copy link
Contributor Author

no tests as previewImage() requires user input to dismiss

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants