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

Missing avatars breaking layout #4343

Closed
jdarwood007 opened this Issue Sep 30, 2017 · 13 comments

Comments

Projects
None yet
7 participants
@jdarwood007
Member

jdarwood007 commented Sep 30, 2017

image

Can we make that look better when the avatar is missing?

@Gwenwyfar

This comment has been minimized.

Show comment
Hide comment
@Gwenwyfar

Gwenwyfar Sep 30, 2017

Collaborator

I'll look into it (unless @Antes wants to do this one)

Collaborator

Gwenwyfar commented Sep 30, 2017

I'll look into it (unless @Antes wants to do this one)

@Antes

This comment has been minimized.

Show comment
Hide comment
@Antes

Antes Sep 30, 2017

Contributor

Doesn`t it require to show Default Avatar if user missing avatar ?

Contributor

Antes commented Sep 30, 2017

Doesn`t it require to show Default Avatar if user missing avatar ?

@jdarwood007

This comment has been minimized.

Show comment
Hide comment
@jdarwood007

jdarwood007 Oct 1, 2017

Member

The files may have been missing or the url bad. I didn't see why it wasn't loading the avatar, just noticed what the layout looked like when it was.
Its hard to click the link to the PM when its like this as well.

Member

jdarwood007 commented Oct 1, 2017

The files may have been missing or the url bad. I didn't see why it wasn't loading the avatar, just noticed what the layout looked like when it was.
Its hard to click the link to the PM when its like this as well.

@Oldiesmann Oldiesmann added this to the RC 1 milestone Oct 1, 2017

@sbulen

This comment has been minimized.

Show comment
Hide comment
@sbulen

sbulen Oct 2, 2017

Contributor

When doing an upgrade, you MUST combine/overwrite the old folders with the new ones, to get the proper default icons. They are different than in 2.0.

Contributor

sbulen commented Oct 2, 2017

When doing an upgrade, you MUST combine/overwrite the old folders with the new ones, to get the proper default icons. They are different than in 2.0.

@illori

This comment has been minimized.

Show comment
Hide comment
@illori

illori Oct 2, 2017

Contributor

combine/overwrite which old folders with what new ones?

Contributor

illori commented Oct 2, 2017

combine/overwrite which old folders with what new ones?

@Antes

This comment has been minimized.

Show comment
Hide comment
@Antes

Antes Oct 2, 2017

Contributor

Default avatar introduced in SMF 2.1, so no, overwrite does not required. Its a code related issue. While unread_alerts fetches the default avatar properly (?), unread_messages does not.

Contributor

Antes commented Oct 2, 2017

Default avatar introduced in SMF 2.1, so no, overwrite does not required. Its a code related issue. While unread_alerts fetches the default avatar properly (?), unread_messages does not.

@sbulen

This comment has been minimized.

Show comment
Hide comment
@sbulen

sbulen Oct 2, 2017

Contributor

Maybe i could have worded that better. Since the defaults images are new, they must find their way into the existing avatar & custom avatar folders during an upgrade. I.e., by copying the new folders that have the default images on top of the existing folders (merging the contents).

In addition, the custom avatar folder did not have a fixed/predetermined name in 2.0. Many (most? all?) users will have custom avatar folders named something other than custom_avatar.

I found it easiest to rename my "avs" folder to custom_avatar in 2.0, prior to the upgrade.

Upgrade instructions will need to address these needs or we will have lots of "missing" avatars. This issue would not have happened had a proper upgrade occured.

Contributor

sbulen commented Oct 2, 2017

Maybe i could have worded that better. Since the defaults images are new, they must find their way into the existing avatar & custom avatar folders during an upgrade. I.e., by copying the new folders that have the default images on top of the existing folders (merging the contents).

In addition, the custom avatar folder did not have a fixed/predetermined name in 2.0. Many (most? all?) users will have custom avatar folders named something other than custom_avatar.

I found it easiest to rename my "avs" folder to custom_avatar in 2.0, prior to the upgrade.

Upgrade instructions will need to address these needs or we will have lots of "missing" avatars. This issue would not have happened had a proper upgrade occured.

@jdarwood007

This comment has been minimized.

Show comment
Hide comment
@jdarwood007

jdarwood007 Oct 2, 2017

Member

While that fixes it for included avatars, external avatars that are missing could also cause this template issue.

Member

jdarwood007 commented Oct 2, 2017

While that fixes it for included avatars, external avatars that are missing could also cause this template issue.

@sbulen

This comment has been minimized.

Show comment
Hide comment
@sbulen

sbulen Oct 3, 2017

Contributor

Correct. The code should be fixed.

Just pointing out that the upgrade process & instructions are an important factor for rollout, or we will see a lot of missing avatars.

Contributor

sbulen commented Oct 3, 2017

Correct. The code should be fixed.

Just pointing out that the upgrade process & instructions are an important factor for rollout, or we will see a lot of missing avatars.

@Gwenwyfar

This comment has been minimized.

Show comment
Hide comment
@Gwenwyfar

Gwenwyfar Oct 28, 2017

Collaborator

I have submitted a fix, but we may need to expand it to some other places where images/avatars are used that may have the same problem.

Collaborator

Gwenwyfar commented Oct 28, 2017

I have submitted a fix, but we may need to expand it to some other places where images/avatars are used that may have the same problem.

@Gwenwyfar

This comment has been minimized.

Show comment
Hide comment
@Gwenwyfar

Gwenwyfar Oct 28, 2017

Collaborator

So we have the same alt text displaying on topics, profile and user menu, although the text doesn't break anything on those.

Collaborator

Gwenwyfar commented Oct 28, 2017

So we have the same alt text displaying on topics, profile and user menu, although the text doesn't break anything on those.

@albertlast albertlast referenced this issue Jan 14, 2018

Open

RC1 Checklist #4485

10 of 13 tasks complete
@Gwenwyfar

This comment has been minimized.

Show comment
Hide comment
@Gwenwyfar

Gwenwyfar Feb 2, 2018

Collaborator

There seems to have been some confusion on this so just to clarify... I'm only getting a few css "fixes" up that were in the previous PR, someone else should take care of this issue.

Collaborator

Gwenwyfar commented Feb 2, 2018

There seems to have been some confusion on this so just to clarify... I'm only getting a few css "fixes" up that were in the previous PR, someone else should take care of this issue.

@albertlast

This comment has been minimized.

Show comment
Hide comment
@albertlast

albertlast Feb 18, 2018

Collaborator

When i understand correctly are more than css fixed not needed?!
And second guess would be that this is than enough:

.top_menu.scrollable .avatar {
	width: 40px;
	height: 40px;
	object-fit: scale-down;
	margin: 4px;
	display: inline-block;
	color: transparent; /* For broken images */
}
/* This will only show when the image does not load */
.top_menu.scrollable .avatar:before {
	content: "";
	display: block;
	background: url(./avatars/default.png) no-repeat center / 100%;
	opacity: 0.5;
	height: 40px;
}
Collaborator

albertlast commented Feb 18, 2018

When i understand correctly are more than css fixed not needed?!
And second guess would be that this is than enough:

.top_menu.scrollable .avatar {
	width: 40px;
	height: 40px;
	object-fit: scale-down;
	margin: 4px;
	display: inline-block;
	color: transparent; /* For broken images */
}
/* This will only show when the image does not load */
.top_menu.scrollable .avatar:before {
	content: "";
	display: block;
	background: url(./avatars/default.png) no-repeat center / 100%;
	opacity: 0.5;
	height: 40px;
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment