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

feat(img): add ionError event #17134

Merged
merged 7 commits into from Mar 27, 2019

Conversation

Projects
None yet
5 participants
@pickfire
Copy link
Contributor

commented Jan 16, 2019

closes #16947

Short description of what this resolves:

Propagate ionError event for img

Changes proposed in this pull request:

Ionic Version: 4.6.0

Fixes: #16947

@@ -77,6 +87,7 @@ export class Img implements ComponentInterface {
src={this.loadSrc}
alt={this.alt}
decoding="async"
onError={this.loadError}

This comment has been minimized.

Copy link
@manucorporat

manucorporat Jan 16, 2019

Member

wondering why we can't just use onError directly

This comment has been minimized.

Copy link
@pickfire

pickfire Jan 17, 2019

Author Contributor

That error trigger even before the image start loading as there is no src.

This comment has been minimized.

Copy link
@paulstelzer

paulstelzer Jan 17, 2019

Collaborator

@pickfire I think manu means removing onError={this.loadError} and instead using the Listen decorator:

@Listen('error')
onImgError(ev: Event) {
 // Your code from loadError
}

This comment has been minimized.

Copy link
@pickfire

pickfire Jan 18, 2019

Author Contributor

I tested that but it does not seem to work.

  /** Emitted when img failed to load */
  @Event() ionError!: EventEmitter<void>;

  @Listen('error')
  onImgError() {
    this.ionError.emit();
  }
  ...
  render() {
    return (
      <img
        src={this.loadSrc}
        alt={this.alt}
        decoding="async"
        // onError={this.loadError}
      />
    );
  }

This comment has been minimized.

Copy link
@liamdebeasi

liamdebeasi Mar 26, 2019

Member

I believe this is because the error event does not bubble: https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onerror

@SamVerschueren

This comment has been minimized.

Copy link

commented Mar 20, 2019

Can this PR be merged? It's really blocking us because we can't implement a fallback image when image loading fails. I just want to know if I should put effort in migrating everything to a regular img tag or if I can wait until this is merged and released.

@pickfire

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2019

@SamVerschueren Same, it's blocking for us as well.

I am not sure whether this PR has been merged yet though, Ionic pull requests are weird.

liamdebeasi added some commits Mar 22, 2019

@liamdebeasi
Copy link
Member

left a comment

Looks good! Just a few small changes, and we'll be good to go.

Show resolved Hide resolved core/src/components/img/img.tsx Outdated
Show resolved Hide resolved core/src/components/img/img.tsx Outdated
@@ -77,6 +87,7 @@ export class Img implements ComponentInterface {
src={this.loadSrc}
alt={this.alt}
decoding="async"
onError={this.loadError}

This comment has been minimized.

Copy link
@liamdebeasi

liamdebeasi Mar 26, 2019

Member

I believe this is because the error event does not bubble: https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onerror

liamdebeasi added some commits Mar 27, 2019

@liamdebeasi
Copy link
Member

left a comment

Great job! 👍

@liamdebeasi
Copy link
Member

left a comment

Thanks for all the work! 🎉

edit: not sure why I re-approved... but I did!

@liamdebeasi liamdebeasi merged commit 04f931f into ionic-team:master Mar 27, 2019

1 check passed

build Workflow: build
Details
@pickfire

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2019

@liamdebeasi Thanks a lot for merging this. 😄

Kiku-git added a commit to Kiku-git/ionic that referenced this pull request May 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.