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

Alt tag on frontpage elements #1050

Closed
Frenchomatic opened this issue Aug 14, 2017 · 4 comments
Closed

Alt tag on frontpage elements #1050

Frenchomatic opened this issue Aug 14, 2017 · 4 comments
Assignees
Milestone

Comments

@Frenchomatic
Copy link

Frenchomatic commented Aug 14, 2017

Related to #1049 but not quite the same issue (may as well fix all at the same time):

I think you should do what you have done on other pages in frontpage_elements and that is use the proper alt tag and not the post title. (i.e. your function ampforwp_thumbnail_alt()). No real reason to make the front page different. This works for me but you can obviously do it differently or condition it on the post title if alt is empty.

<figure class="amp-wp-article-featured-image wp-caption"> <?php  
				$thumb_id = get_post_thumbnail_id($post_id);
				$image = wp_get_attachment_image_src( $thumb_id, 'medium' ); 
				$caption = get_the_post_thumbnail_caption( $post_id ); 
				$alt = esc_attr(get_post_meta( $thumb_id, '_wp_attachment_image_alt', true));
				?>
				<amp-img src="<?php echo $image[0]; ?>" width="<?php echo $image[1]; ?>" height="<?php echo $image[2]; ?>" layout=responsive alt="<?php echo $alt; ?>" >  </amp-img>

I already do something similar on the logo in header-bar.php because the logo has an alt tag defined.

MARQAS added a commit that referenced this issue Aug 17, 2017
@MARQAS MARQAS self-assigned this Aug 17, 2017
@MARQAS MARQAS added this to the 0.9.59 milestone Aug 17, 2017
@Frenchomatic
Copy link
Author

Thanks Marqas - lots of progress.

@Frenchomatic
Copy link
Author

Also, as you can see on the forum people have an issue with blurred images. Why not use the same featured image with screen set on the front page as on all other pages / posts. I know it takes a bit more resource but it is minuscule and it avoids people posting on the forum about it.

MARQAS added a commit that referenced this issue Aug 18, 2017
Forgot to select these lines while committing(my bad 😐 )
@MARQAS
Copy link
Contributor

MARQAS commented Aug 19, 2017

Hi @Frenchomatic ,
I have created a separate ticket about the featured, we will test it and will definitely solve this problem
#1063

@ahmedkaludi
Copy link
Owner

Reviewed

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

No branches or pull requests

3 participants