WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#27959 closed enhancement (fixed)

get_uploaded_header_images(); should provide alternate text of that image

Reported by: francoeurdavid Owned by: obenland
Milestone: 4.3 Priority: normal
Severity: normal Version: 3.2
Component: Customize Keywords: has-patch
Focuses: template Cc:

Description

That method should provide the Alternate Text of each images along with all the stuff that is already provided.

Same code is up from 3.2.0.

The method is located in wp-includes/themes.php
2 lines of code are to be added.

Attachments (4)

27959-patch.diff (856 bytes) - added by francoeurdavid 5 years ago.
patch
27959.2.diff (868 bytes) - added by voldemortensen 5 years ago.
27959.diff (869 bytes) - added by valendesigns 4 years ago.
27959.3.diff (2.1 KB) - added by valendesigns 4 years ago.

Download all attachments as: .zip

Change History (24)

@francoeurdavid
5 years ago

patch

#1 @francoeurdavid
5 years ago

I uploaded a patch.

#2 @helen
5 years ago

  • Component changed from General to Customize
  • Focuses template added
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Type changed from feature request to enhancement
  • Version changed from trunk to 3.2

Seems reasonable.

#3 @voldemortensen
5 years ago

Updated patch to current trunk.

#4 @celloexpressions
5 years ago

Related/possible duplicate: #15926.

This ticket was mentioned in Slack in #core-customize by valendesigns. View the logs.


4 years ago

@valendesigns
4 years ago

#6 @valendesigns
4 years ago

  • Keywords commit added
  • Milestone changed from Future Release to 4.3
  • Owner set to valendesigns
  • Status changed from new to assigned

I've refreshed the patch and fixed a typo, plus tested that it works as expected. Looks good and moving for commit.

#7 @obenland
4 years ago

  • Keywords commit removed

What are we going to do with the additional information?

#8 follow-up: @valendesigns
4 years ago

@obenland I think the argument here is that the data should be provided regardless of if Core uses it or not. Someone could directly use the get_uploaded_header_images function and there is no alternative text attached to the return array. It should be and it doesn't hurt to add it. As well, Core is doing it wrong when it comes to adding alt text to the header images as it's using the description field, which is very confusing. This patch would allow us to fix that in a future ticket.

#9 in reply to: ↑ 8 @obenland
4 years ago

Replying to valendesigns:

@obenland I think the argument here is that the data should be provided regardless of if Core uses it or not.

Playing devil's advocate: Why?

Can we find a sane use case for it in core?

#10 follow-up: @valendesigns
4 years ago

In Core we currently use the description field to set the alternate text for the header images even though there is a field that is specific for this very purpose in the media uploaded that is not being used. Do you want line numbers of where we could make these changes or does that make enough sense to move this forward?

#11 in reply to: ↑ 10 @obenland
4 years ago

Replying to valendesigns:

Do you want line numbers of where we could make these changes or does that make enough sense to move this forward?

Sounds good, do you want to create a patch for it?

#12 @obenland
4 years ago

@valendesigns, any movement on this?

#13 @valendesigns
4 years ago

My bad! I'll get it sorted this weekend.

#14 follow-up: @valendesigns
4 years ago

@obenland I think that this patch should remain as is and not change other parts of Core. This is more for convenience and would give theme authors access to the alt text by default.

Though I think after looking at other parts of Core the array key should be set to alt_text instead of alt, which is what the get_uploaded_header_images() method in the Custom_Image_Header class uses. I can upload another patch but wanted to get your opinion first.

#15 in reply to: ↑ 14 @obenland
4 years ago

Replying to valendesigns:

@obenland I think that this patch should remain as is and not change other parts of Core. This is more for convenience and would give theme authors access to the alt text by default.

I think we should check for the alt text here and make description the fallback, like it's done in the customizer.

Though I think after looking at other parts of Core the array key should be set to alt_text instead of alt, which is what the get_uploaded_header_images() method in the Custom_Image_Header class uses. I can upload another patch but wanted to get your opinion first.

No objections to alt_text.

@valendesigns
4 years ago

#16 follow-up: @valendesigns
4 years ago

I've updated custom-header.php so it uses the alt text with a description fallback. Though if I navigate to custom-header.php in the admin I get a black screen, so I'm not sure how I should be testing that it works correctly.

#17 @obenland
4 years ago

  • Owner changed from valendesigns to obenland
  • Status changed from assigned to accepted

#18 in reply to: ↑ 16 @obenland
4 years ago

Replying to valendesigns:

I've updated custom-header.php so it uses the alt text with a description fallback. Though if I navigate to custom-header.php in the admin I get a black screen, so I'm not sure how I should be testing that it works correctly.

It's testable at /wp-admin/themes.php?page=custom-header.

#19 @obenland
4 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 32998:

Provide alt text for uploaded header images.

The custom header screen will use it rather then the description,
if an alt text is set.

Props francoeurdavid, voldemortensen, valendesigns.
Fixes #27959.

#20 @francoeurdavid
4 years ago

Thanks to you guys, I barely did anything. I'm impressed by all the work you've done for such a small fixes. I realize there are a lot of things I didn't think about.

Thanks again for the good job, and even if I didn't say anything, I was following up closely :)

Note: See TracTickets for help on using tickets.