Skip to content

[Finishes #98332908] setImageURL shouldn't resize#3019

Merged
Bjvanminnen merged 2 commits into
stagingfrom
imageResize
Jul 6, 2015
Merged

[Finishes #98332908] setImageURL shouldn't resize#3019
Bjvanminnen merged 2 commits into
stagingfrom
imageResize

Conversation

@Bjvanminnen

Copy link
Copy Markdown
Contributor

We would have API code call setImageURL. This would result on us hitting the onload that we still had floating around from design mode on the element, which does a resize. We don't want to hit this onload other than the once, so clear it out as soon as we hit it.

@joshlory

joshlory commented Jul 3, 2015

Copy link
Copy Markdown
Contributor

Nit: maybe delete backgroundImage.onload but what you have is fine. LGTM!

@Bjvanminnen

Copy link
Copy Markdown
Contributor Author

Seems like there might still be an issue here, so I'm going to spend a little more time looking at this Monday.

@Bjvanminnen

Copy link
Copy Markdown
Contributor Author

The original fix wasn't working, because over the course of run/reset we would apparently end up calling onPropertyChange for image. This would then create a new onload function. Each instance would only be called once, but because we could have multiple instances, we were still getting into cases where setting via the API would change the size.

New approach is that we only create our onload function when setting to a new URL. Tested the following cases:

  • Set image via design mode. Resize. Change image via API. Image remains the same size.
  • Set image via design mode. Set to a differently sized image. Image resizes

@Bjvanminnen

Copy link
Copy Markdown
Contributor Author

Note: Diff looks bigger than it is. Really I'm just wrapping the onload = function() in an if block.

@islemaster

Copy link
Copy Markdown
Contributor

LGTM.

Bjvanminnen added a commit that referenced this pull request Jul 6, 2015
[Finishes #98332908] setImageURL shouldn't resize
@Bjvanminnen Bjvanminnen merged commit 29d37cd into staging Jul 6, 2015
@Bjvanminnen Bjvanminnen deleted the imageResize branch July 6, 2015 19:03
deploy-code-org added a commit that referenced this pull request Jul 6, 2015
commit 29d37cd
Merge: 3df4252 a42a3da
Author: Bjvanminnen <Bjvanminnen@gmail.com>
Date:   Mon Jul 6 12:03:12 2015 -0700

    Merge pull request #3019 from code-dot-org/imageResize

    [Finishes #98332908] setImageURL shouldn't resize

commit 3df4252
Author: Continuous Integration <dev@code.org>
Date:   Mon Jul 6 18:33:40 2015 +0000

    Automatically built.

    commit 370f507
    Author: Brent Van Minnen <bjvanminnen@gmail.com>
    Date:   Mon Jul 6 11:35:10 2015 -0700

        Revert "Merge pull request #2992 from code-dot-org/apps-digest"

        This reverts commit 6ad6441, reversing
        changes made to fac267e.

commit 370f507
Author: Brent Van Minnen <bjvanminnen@gmail.com>
Date:   Mon Jul 6 11:35:10 2015 -0700

    Revert "Merge pull request #2992 from code-dot-org/apps-digest"

    This reverts commit 6ad6441, reversing
    changes made to fac267e.
deploy-code-org added a commit that referenced this pull request Jul 6, 2015
commit a05fd0f
Merge: db4cf08 20b544a
Author: Bjvanminnen <Bjvanminnen@gmail.com>
Date:   Mon Jul 6 12:28:54 2015 -0700

    Merge pull request #3023 from code-dot-org/sharingCodeless

    able to properly share project without code

commit db4cf08
Author: Continuous Integration <dev@code.org>
Date:   Mon Jul 6 19:10:55 2015 +0000

    Automatically built.

    commit 29d37cd
    Merge: 3df4252 a42a3da
    Author: Bjvanminnen <Bjvanminnen@gmail.com>
    Date:   Mon Jul 6 12:03:12 2015 -0700

        Merge pull request #3019 from code-dot-org/imageResize

        [Finishes #98332908] setImageURL shouldn't resize

    commit 3df4252
    Author: Continuous Integration <dev@code.org>
    Date:   Mon Jul 6 18:33:40 2015 +0000

        Automatically built.

        commit 370f507
        Author: Brent Van Minnen <bjvanminnen@gmail.com>
        Date:   Mon Jul 6 11:35:10 2015 -0700

            Revert "Merge pull request #2992 from code-dot-org/apps-digest"

            This reverts commit 6ad6441, reversing
            changes made to fac267e.

    commit 370f507
    Author: Brent Van Minnen <bjvanminnen@gmail.com>
    Date:   Mon Jul 6 11:35:10 2015 -0700

        Revert "Merge pull request #2992 from code-dot-org/apps-digest"

        This reverts commit 6ad6441, reversing
        changes made to fac267e.
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.

3 participants