Commit 36f7b212 authored by Tom Quirk's avatar Tom Quirk

Fix imageLoading bug when scrolling back to design

imageLoading was being set incorrectly (every time the item appeared).
This meant that even if the image was not loading, the loading
spinner would appear if the image appeared in view again.

This commit changes the logic so that we only set imageLoading if the
image has never appeared in view.

This commit also adds and reorganises some tests to ensure this path is
covered.
parent e86e63fa
...@@ -54,7 +54,7 @@ export default { ...@@ -54,7 +54,7 @@ export default {
return { return {
imageLoading: true, imageLoading: true,
imageError: false, imageError: false,
isInView: false, wasInView: false,
}; };
}, },
computed: { computed: {
...@@ -84,13 +84,13 @@ export default { ...@@ -84,13 +84,13 @@ export default {
return n__('%d comment', '%d comments', this.notesCount); return n__('%d comment', '%d comments', this.notesCount);
}, },
imageLink() { imageLink() {
return this.isInView ? this.imageV432x230 || this.image : ''; return this.wasInView ? this.imageV432x230 || this.image : '';
}, },
showLoadingSpinner() { showLoadingSpinner() {
return this.imageLoading || this.isUploading; return this.imageLoading || this.isUploading;
}, },
showImageErrorIcon() { showImageErrorIcon() {
return this.imageError && this.isInView; return this.wasInView && this.imageError;
}, },
showImage() { showImage() {
return !this.showLoadingSpinner && !this.showImageErrorIcon; return !this.showLoadingSpinner && !this.showImageErrorIcon;
...@@ -106,7 +106,13 @@ export default { ...@@ -106,7 +106,13 @@ export default {
this.imageError = true; this.imageError = true;
}, },
onAppear() { onAppear() {
this.isInView = true; // do nothing if image has previously
// been in view
if (this.wasInView) {
return;
}
this.wasInView = true;
this.imageLoading = true; this.imageLoading = true;
}, },
}, },
......
---
title: Fix imageLoading bug when scrolling back to design
merge_request: 29223
author:
type: fixed
// Jest Snapshot v1, https://goo.gl/fbAQLP // Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`Design management list item component when item appears in view renders media broken icon when image onerror triggered 1`] = ` exports[`Design management list item component when item appears in view after image is loaded renders media broken icon when image onerror triggered 1`] = `
<gl-icon-stub <gl-icon-stub
class="text-secondary" class="text-secondary"
name="media-broken" name="media-broken"
......
import { createLocalVue, shallowMount } from '@vue/test-utils'; import { createLocalVue, shallowMount } from '@vue/test-utils';
import { GlIcon, GlIntersectionObserver } from '@gitlab/ui'; import { GlIcon, GlLoadingIcon, GlIntersectionObserver } from '@gitlab/ui';
import VueRouter from 'vue-router'; import VueRouter from 'vue-router';
import Item from 'ee/design_management/components/list/item.vue'; import Item from 'ee/design_management/components/list/item.vue';
...@@ -60,36 +60,63 @@ describe('Design management list item component', () => { ...@@ -60,36 +60,63 @@ describe('Design management list item component', () => {
describe('when item appears in view', () => { describe('when item appears in view', () => {
let image; let image;
let glIntersectionObserver;
beforeEach(() => { beforeEach(() => {
createComponent(); createComponent();
image = wrapper.find('img'); image = wrapper.find('img');
glIntersectionObserver = wrapper.find(GlIntersectionObserver);
expect(image.attributes('src')).toBe(''); expect(image.attributes('src')).toBe('');
wrapper.find(GlIntersectionObserver).vm.$emit('appear'); glIntersectionObserver.vm.$emit('appear');
return wrapper.vm.$nextTick(); return wrapper.vm.$nextTick();
}); });
it('renders an image', () => { describe('before image is loaded', () => {
expect(image.attributes('src')).toBe('http://via.placeholder.com/300'); it('renders loading spinner', () => {
expect(wrapper.find(GlLoadingIcon)).toExist();
});
}); });
it('renders media broken icon when image onerror triggered', () => { describe('after image is loaded', () => {
image.trigger('error'); beforeEach(() => {
return wrapper.vm.$nextTick().then(() => { image.trigger('load');
expect(image.isVisible()).toBe(false); return wrapper.vm.$nextTick();
expect(wrapper.find(GlIcon).element).toMatchSnapshot();
}); });
});
describe('when imageV432x230 and image provided', () => { it('renders an image', () => {
it('renders imageV432x230 image', () => { expect(image.attributes('src')).toBe('http://via.placeholder.com/300');
const mockSrc = 'mock-imageV432x230-url'; expect(image.isVisible()).toBe(true);
wrapper.setProps({ imageV432x230: mockSrc }); });
it('renders media broken icon when image onerror triggered', () => {
image.trigger('error');
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
expect(image.attributes('src')).toBe(mockSrc); expect(image.isVisible()).toBe(false);
expect(wrapper.find(GlIcon).element).toMatchSnapshot();
});
});
describe('when imageV432x230 and image provided', () => {
it('renders imageV432x230 image', () => {
const mockSrc = 'mock-imageV432x230-url';
wrapper.setProps({ imageV432x230: mockSrc });
return wrapper.vm.$nextTick().then(() => {
expect(image.attributes('src')).toBe(mockSrc);
});
});
});
describe('when image disappears from view and then reappears', () => {
beforeEach(() => {
glIntersectionObserver.vm.$emit('appear');
return wrapper.vm.$nextTick();
});
it('renders an image', () => {
expect(image.isVisible()).toBe(true);
}); });
}); });
}); });
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment