root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Send resize parameters for avatars

This commit is contained in:
Tim Zallmann 2018-08-07 06:04:33 +00:00 committed by Sean McGivern
parent dd627072b3
commit fac4f50cf6
14 changed files with 136 additions and 122 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
app/assets/javascripts/lazy_loader.js
View File

@ -1,6 +1,7 @@
import _ from 'underscore';
export const placeholderImage = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==';
export const placeholderImage =
'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==';
const SCROLL_THRESHOLD = 300;
export default class LazyLoader {
@ -48,7 +49,7 @@ export default class LazyLoader {
const visHeight = scrollTop + window.innerHeight + SCROLL_THRESHOLD;
// Loading Images which are in the current viewport or close to them
this.lazyImages = this.lazyImages.filter((selectedImage) => {
this.lazyImages = this.lazyImages.filter(selectedImage => {
if (selectedImage.getAttribute('data-src')) {
const imgBoundRect = selectedImage.getBoundingClientRect();
const imgTop = scrollTop + imgBoundRect.top;
@ -66,7 +67,18 @@ export default class LazyLoader {
}
static loadImage(img) {
if (img.getAttribute('data-src')) {
img.setAttribute('src', img.getAttribute('data-src'));
let imgUrl = img.getAttribute('data-src');
// Only adding width + height for avatars for now
if (imgUrl.indexOf('/avatar/') > -1 && imgUrl.indexOf('?') === -1) {
let targetWidth = null;
if (img.getAttribute('width')) {
targetWidth = img.getAttribute('width');
} else {
targetWidth = img.width;
}
if (targetWidth) imgUrl += `?width=${targetWidth}`;
}
img.setAttribute('src', imgUrl);
img.removeAttribute('data-src');
img.classList.remove('lazy');
img.classList.add('js-lazy-loaded');

5
app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_image.vue
View File

@ -1,5 +1,4 @@
<script>
/* This is a re-usable vue component for rendering a user avatar that
does not need to link to the user's profile. The image and an optional
tooltip can be configured by props passed to this component.
@ -67,7 +66,9 @@ export default {
// we provide an empty string when we use it inside user avatar link.
// In both cases we should render the defaultAvatarUrl
sanitizedSource() {
return this.imgSrc === '' || this.imgSrc === null ? defaultAvatarUrl : this.imgSrc;
let baseSrc = this.imgSrc === '' || this.imgSrc === null ? defaultAvatarUrl : this.imgSrc;
if (baseSrc.indexOf('?') === -1) baseSrc += `?width=${this.size}`;
return baseSrc;
},
resultantSrcAttribute() {
return this.lazy ? placeholderImage : this.sanitizedSource;

7
app/models/concerns/avatarable.rb
View File

@ -19,7 +19,7 @@ module Avatarable
# We use avatar_path instead of overriding avatar_url because of carrierwave.
# See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11001/diffs#note_28659864
avatar_path(only_path: args.fetch(:only_path, true)) || super
avatar_path(only_path: args.fetch(:only_path, true), size: args[:size]) || super
end
def retrieve_upload(identifier, paths)
@ -40,12 +40,13 @@ module Avatarable
end
end
def avatar_path(only_path: true)
def avatar_path(only_path: true, size: nil)
return unless self[:avatar].present?
asset_host = ActionController::Base.asset_host
use_asset_host = asset_host.present?
use_authentication = respond_to?(:public?) && !public?
query_params = size&.nonzero? ? "?width=#{size}" : ""
# Avatars for private and internal groups and projects require authentication to be viewed,
# which means they can only be served by Rails, on the regular GitLab host.
@ -64,7 +65,7 @@ module Avatarable
url_base << gitlab_config.relative_url_root
end
url_base + avatar.local_url
url_base + avatar.local_url + query_params
end
# Path that is persisted in the tracking Upload model. Used to fetch the

2
app/views/admin/projects/_projects.html.haml
View File

@ -20,7 +20,7 @@
= link_to(admin_namespace_project_path(project.namespace, project)) do
.dash-project-avatar
.avatar-container.s40
= project_icon(project, alt: '', class: 'avatar project-avatar s40')
= project_icon(project, alt: '', class: 'avatar project-avatar s40', width: 40, height: 40)
%span.project-full-name
%span.namespace-name
- if project.namespace

2
app/views/layouts/nav/sidebar/_project.html.haml
View File

@ -4,7 +4,7 @@
.context-header
= link_to project_path(@project), title: @project.name do
.avatar-container.s40.project-avatar
= project_icon(@project, alt: @project.name, class: 'avatar s40 avatar-tile')
= project_icon(@project, alt: @project.name, class: 'avatar s40 avatar-tile', width: 40, height: 40)
.sidebar-context-title
= @project.name
%ul.sidebar-top-level-items

2
app/views/projects/_home_panel.html.haml
View File

@ -3,7 +3,7 @@
.project-home-panel.text-center{ class: ("empty-project" if empty_repo) }
.limit-container-width{ class: container_class }
.avatar-container.s70.project-avatar
= project_icon(@project, alt: @project.name, class: 'avatar s70 avatar-tile')
= project_icon(@project, alt: @project.name, class: 'avatar s70 avatar-tile', width: 70, height: 70)
%h1.project-title.qa-project-name
= @project.name
%span.visibility-icon.has-tooltip{ data: { container: 'body' }, title: visibility_icon_description(@project) }

2
app/views/projects/edit.html.haml
View File

@ -51,7 +51,7 @@
.form-group
- if @project.avatar?
.avatar-container.s160.append-bottom-15
= project_icon(@project.full_path, alt: '', class: 'avatar project-avatar s160')
= project_icon(@project.full_path, alt: '', class: 'avatar project-avatar s160', width: 160, height: 160)
- if @project.avatar_in_git
%p.light
= _("Project avatar in repository: %{link}").html_safe % { link: @project.avatar_in_git }

2
app/views/shared/projects/_project.html.haml
View File

@ -19,7 +19,7 @@
- if project.creator && use_creator_avatar
= image_tag avatar_icon_for_user(project.creator, 40), class: "avatar s40", alt:''
- else
= project_icon(project, alt: '', class: 'avatar project-avatar s40')
= project_icon(project, alt: '', class: 'avatar project-avatar s40', width: 40, height: 40)
.project-details
%h3.prepend-top-0.append-bottom-0
= link_to project_path(project), class: 'text-plain' do

2
spec/features/uploads/user_uploads_avatar_to_profile_spec.rb
View File

@ -15,7 +15,7 @@ describe 'User uploads avatar to profile' do
visit user_path(user)
expect(page).to have_selector(%Q(img[data-src$="/uploads/-/system/user/avatar/#{user.id}/dk.png"]))
expect(page).to have_selector(%Q(img[data-src$="/uploads/-/system/user/avatar/#{user.id}/dk.png?width=90"]))
# Cheating here to verify something that isn't user-facing, but is important
expect(user.reload.avatar.file).to exist

154
spec/javascripts/boards/issue_card_spec.js
View File

@ -69,109 +69,100 @@ describe('Issue card component', () => {
});
it('renders issue title', () => {
expect(
component.$el.querySelector('.board-card-title').textContent,
).toContain(issue.title);
expect(component.$el.querySelector('.board-card-title').textContent).toContain(issue.title);
});
it('includes issue base in link', () => {
expect(
component.$el.querySelector('.board-card-title a').getAttribute('href'),
).toContain('/test');
expect(component.$el.querySelector('.board-card-title a').getAttribute('href')).toContain(
'/test',
);
});
it('includes issue title on link', () => {
expect(
component.$el.querySelector('.board-card-title a').getAttribute('title'),
).toBe(issue.title);
expect(component.$el.querySelector('.board-card-title a').getAttribute('title')).toBe(
issue.title,
);
});
it('does not render confidential icon', () => {
expect(
component.$el.querySelector('.fa-eye-flash'),
).toBeNull();
expect(component.$el.querySelector('.fa-eye-flash')).toBeNull();
});
it('renders confidential icon', (done) => {
it('renders confidential icon', done => {
component.issue.confidential = true;
Vue.nextTick(() => {
expect(
component.$el.querySelector('.confidential-icon'),
).not.toBeNull();
expect(component.$el.querySelector('.confidential-icon')).not.toBeNull();
done();
});
});
it('renders issue ID with #', () => {
expect(
component.$el.querySelector('.board-card-number').textContent,
).toContain(`#${issue.id}`);
expect(component.$el.querySelector('.board-card-number').textContent).toContain(`#${issue.id}`);
});
describe('assignee', () => {
it('does not render assignee', () => {
expect(
component.$el.querySelector('.board-card-assignee .avatar'),
).toBeNull();
expect(component.$el.querySelector('.board-card-assignee .avatar')).toBeNull();
});
describe('exists', () => {
beforeEach((done) => {
beforeEach(done => {
component.issue.assignees = [user];
Vue.nextTick(() => done());
});
it('renders assignee', () => {
expect(
component.$el.querySelector('.board-card-assignee .avatar'),
).not.toBeNull();
expect(component.$el.querySelector('.board-card-assignee .avatar')).not.toBeNull();
});
it('sets title', () => {
expect(
component.$el.querySelector('.board-card-assignee img').getAttribute('data-original-title'),
component.$el
.querySelector('.board-card-assignee img')
.getAttribute('data-original-title'),
).toContain(`Assigned to ${user.name}`);
});
it('sets users path', () => {
expect(
component.$el.querySelector('.board-card-assignee a').getAttribute('href'),
).toBe('/test');
expect(component.$el.querySelector('.board-card-assignee a').getAttribute('href')).toBe(
'/test',
);
});
it('renders avatar', () => {
expect(
component.$el.querySelector('.board-card-assignee img'),
).not.toBeNull();
expect(component.$el.querySelector('.board-card-assignee img')).not.toBeNull();
});
});
describe('assignee default avatar', () => {
beforeEach((done) => {
component.issue.assignees = [new ListAssignee({
id: 1,
name: 'testing 123',
username: 'test',
}, 'default_avatar')];
beforeEach(done => {
component.issue.assignees = [
new ListAssignee(
{
id: 1,
name: 'testing 123',
username: 'test',
},
'default_avatar',
),
];
Vue.nextTick(done);
});
it('displays defaults avatar if users avatar is null', () => {
expect(
component.$el.querySelector('.board-card-assignee img'),
).not.toBeNull();
expect(
component.$el.querySelector('.board-card-assignee img').getAttribute('src'),
).toBe('default_avatar');
expect(component.$el.querySelector('.board-card-assignee img')).not.toBeNull();
expect(component.$el.querySelector('.board-card-assignee img').getAttribute('src')).toBe(
'default_avatar?width=20',
);
});
});
});
describe('multiple assignees', () => {
beforeEach((done) => {
beforeEach(done => {
component.issue.assignees = [
user,
new ListAssignee({
@ -191,7 +182,8 @@ describe('Issue card component', () => {
name: 'user4',
username: 'user4',
avatar: 'test_image',
})];
}),
];
Vue.nextTick(() => done());
});
@ -201,26 +193,30 @@ describe('Issue card component', () => {
});
describe('more than four assignees', () => {
beforeEach((done) => {
component.issue.assignees.push(new ListAssignee({
id: 5,
name: 'user5',
username: 'user5',
avatar: 'test_image',
}));
beforeEach(done => {
component.issue.assignees.push(
new ListAssignee({
id: 5,
name: 'user5',
username: 'user5',
avatar: 'test_image',
}),
);
Vue.nextTick(() => done());
});
it('renders more avatar counter', () => {
expect(component.$el.querySelector('.board-card-assignee .avatar-counter').innerText).toEqual('+2');
expect(
component.$el.querySelector('.board-card-assignee .avatar-counter').innerText,
).toEqual('+2');
});
it('renders three assignees', () => {
expect(component.$el.querySelectorAll('.board-card-assignee .avatar').length).toEqual(3);
});
it('renders 99+ avatar counter', (done) => {
it('renders 99+ avatar counter', done => {
for (let i = 5; i < 104; i += 1) {
const u = new ListAssignee({
id: i,
@ -232,7 +228,9 @@ describe('Issue card component', () => {
}
Vue.nextTick(() => {
expect(component.$el.querySelector('.board-card-assignee .avatar-counter').innerText).toEqual('99+');
expect(
component.$el.querySelector('.board-card-assignee .avatar-counter').innerText,
).toEqual('99+');
done();
});
});
@ -240,59 +238,51 @@ describe('Issue card component', () => {
});
describe('labels', () => {
beforeEach((done) => {
beforeEach(done => {
component.issue.addLabel(label1);
Vue.nextTick(() => done());
});
it('renders list label', () => {
expect(
component.$el.querySelectorAll('.badge').length,
).toBe(2);
expect(component.$el.querySelectorAll('.badge').length).toBe(2);
});
it('renders label', () => {
const nodes = [];
component.$el.querySelectorAll('.badge').forEach((label) => {
component.$el.querySelectorAll('.badge').forEach(label => {
nodes.push(label.getAttribute('data-original-title'));
});
expect(
nodes.includes(label1.description),
).toBe(true);
expect(nodes.includes(label1.description)).toBe(true);
});
it('sets label description as title', () => {
expect(
component.$el.querySelector('.badge').getAttribute('data-original-title'),
).toContain(label1.description);
expect(component.$el.querySelector('.badge').getAttribute('data-original-title')).toContain(
label1.description,
);
});
it('sets background color of button', () => {
const nodes = [];
component.$el.querySelectorAll('.badge').forEach((label) => {
component.$el.querySelectorAll('.badge').forEach(label => {
nodes.push(label.style.backgroundColor);
});
expect(
nodes.includes(label1.color),
).toBe(true);
expect(nodes.includes(label1.color)).toBe(true);
});
it('does not render label if label does not have an ID', (done) => {
component.issue.addLabel(new ListLabel({
title: 'closed',
}));
it('does not render label if label does not have an ID', done => {
component.issue.addLabel(
new ListLabel({
title: 'closed',
}),
);
Vue.nextTick()
.then(() => {
expect(
component.$el.querySelectorAll('.badge').length,
).toBe(2);
expect(
component.$el.textContent,
).not.toContain('closed');
expect(component.$el.querySelectorAll('.badge').length).toBe(2);
expect(component.$el.textContent).not.toContain('closed');
done();
})

26
spec/javascripts/pipelines/pipeline_url_spec.js
View File

@ -35,7 +35,9 @@ describe('Pipeline Url Component', () => {
},
}).$mount();
expect(component.$el.querySelector('.js-pipeline-url-link').getAttribute('href')).toEqual('foo');
expect(component.$el.querySelector('.js-pipeline-url-link').getAttribute('href')).toEqual(
'foo',
);
expect(component.$el.querySelector('.js-pipeline-url-link span').textContent).toEqual('#1');
});
@ -61,11 +63,11 @@ describe('Pipeline Url Component', () => {
const image = component.$el.querySelector('.js-pipeline-url-user img');
expect(
component.$el.querySelector('.js-pipeline-url-user').getAttribute('href'),
).toEqual(mockData.pipeline.user.web_url);
expect(component.$el.querySelector('.js-pipeline-url-user').getAttribute('href')).toEqual(
mockData.pipeline.user.web_url,
);
expect(image.getAttribute('data-original-title')).toEqual(mockData.pipeline.user.name);
expect(image.getAttribute('src')).toEqual(mockData.pipeline.user.avatar_url);
expect(image.getAttribute('src')).toEqual(`${mockData.pipeline.user.avatar_url}?width=20`);
});
it('should render "API" when no user is provided', () => {
@ -100,7 +102,9 @@ describe('Pipeline Url Component', () => {
}).$mount();
expect(component.$el.querySelector('.js-pipeline-url-latest').textContent).toContain('latest');
expect(component.$el.querySelector('.js-pipeline-url-yaml').textContent).toContain('yaml invalid');
expect(component.$el.querySelector('.js-pipeline-url-yaml').textContent).toContain(
'yaml invalid',
);
expect(component.$el.querySelector('.js-pipeline-url-stuck').textContent).toContain('stuck');
});
@ -121,9 +125,9 @@ describe('Pipeline Url Component', () => {
},
}).$mount();
expect(
component.$el.querySelector('.js-pipeline-url-autodevops').textContent.trim(),
).toEqual('Auto DevOps');
expect(component.$el.querySelector('.js-pipeline-url-autodevops').textContent.trim()).toEqual(
'Auto DevOps',
);
});
it('should render error badge when pipeline has a failure reason set', () => {
@ -142,6 +146,8 @@ describe('Pipeline Url Component', () => {
}).$mount();
expect(component.$el.querySelector('.js-pipeline-url-failure').textContent).toContain('error');
expect(component.$el.querySelector('.js-pipeline-url-failure').getAttribute('data-original-title')).toContain('some reason');
expect(
component.$el.querySelector('.js-pipeline-url-failure').getAttribute('data-original-title'),
).toContain('some reason');
});
});

2
spec/javascripts/vue_shared/components/notes/placeholder_note_spec.js
View File

@ -27,7 +27,7 @@ describe('issue placeholder system note component', () => {
userDataMock.path,
);
expect(vm.$el.querySelector('.user-avatar-link img').getAttribute('src')).toEqual(
userDataMock.avatar_url,
`${userDataMock.avatar_url}?width=40`,
);
});
});

30
spec/javascripts/vue_shared/components/user_avatar/user_avatar_image_spec.js
View File

@ -12,7 +12,7 @@ const DEFAULT_PROPS = {
tooltipPlacement: 'bottom',
};
describe('User Avatar Image Component', function () {
describe('User Avatar Image Component', function() {
let vm;
let UserAvatarImage;
@ -20,37 +20,37 @@ describe('User Avatar Image Component', function () {
UserAvatarImage = Vue.extend(userAvatarImage);
});
describe('Initialization', function () {
beforeEach(function () {
describe('Initialization', function() {
beforeEach(function() {
vm = mountComponent(UserAvatarImage, {
...DEFAULT_PROPS,
}).$mount();
});
it('should return a defined Vue component', function () {
it('should return a defined Vue component', function() {
expect(vm).toBeDefined();
});
it('should have <img> as a child element', function () {
it('should have <img> as a child element', function() {
expect(vm.$el.tagName).toBe('IMG');
expect(vm.$el.getAttribute('src')).toBe(DEFAULT_PROPS.imgSrc);
expect(vm.$el.getAttribute('data-src')).toBe(DEFAULT_PROPS.imgSrc);
expect(vm.$el.getAttribute('src')).toBe(`${DEFAULT_PROPS.imgSrc}?width=99`);
expect(vm.$el.getAttribute('data-src')).toBe(`${DEFAULT_PROPS.imgSrc}?width=99`);
expect(vm.$el.getAttribute('alt')).toBe(DEFAULT_PROPS.imgAlt);
});
it('should properly compute tooltipContainer', function () {
it('should properly compute tooltipContainer', function() {
expect(vm.tooltipContainer).toBe('body');
});
it('should properly render tooltipContainer', function () {
it('should properly render tooltipContainer', function() {
expect(vm.$el.getAttribute('data-container')).toBe('body');
});
it('should properly compute avatarSizeClass', function () {
it('should properly compute avatarSizeClass', function() {
expect(vm.avatarSizeClass).toBe('s99');
});
it('should properly render img css', function () {
it('should properly render img css', function() {
const { classList } = vm.$el;
const containsAvatar = classList.contains('avatar');
const containsSizeClass = classList.contains('s99');
@ -64,21 +64,21 @@ describe('User Avatar Image Component', function () {
});
});
describe('Initialization when lazy', function () {
beforeEach(function () {
describe('Initialization when lazy', function() {
beforeEach(function() {
vm = mountComponent(UserAvatarImage, {
...DEFAULT_PROPS,
lazy: true,
}).$mount();
});
it('should add lazy attributes', function () {
it('should add lazy attributes', function() {
const { classList } = vm.$el;
const lazyClass = classList.contains('lazy');
expect(lazyClass).toBe(true);
expect(vm.$el.getAttribute('src')).toBe(placeholderImage);
expect(vm.$el.getAttribute('data-src')).toBe(DEFAULT_PROPS.imgSrc);
expect(vm.$el.getAttribute('data-src')).toBe(`${DEFAULT_PROPS.imgSrc}?width=99`);
});
});
});

4
spec/models/concerns/avatarable_spec.rb
View File

@ -43,6 +43,10 @@ describe Avatarable do
expect(project.avatar_path(only_path: only_path)).to eq(avatar_path)
end
it 'returns the expected avatar path with width parameter' do
expect(project.avatar_path(only_path: only_path, size: 128)).to eq(avatar_path + "?width=128")
end
context "when avatar is stored remotely" do
before do
stub_uploads_object_storage(AvatarUploader)