WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#29577 closed defect (bug) (fixed)

Twenty Twelve and Thirteen: Use proper way to get customizer settings values

Reported by: obenland Owned by: kdoran
Milestone: 4.1 Priority: normal
Severity: normal Version: 3.5
Component: Bundled Theme Keywords: good-first-bug has-patch commit
Focuses: javascript Cc:

Description (last modified by obenland)

In Twenty Thirteen we check whether the header image is removed in the Customizer JS. We do so by checking the private _wpCustomizeSettings variable.

Let's do it properly.

Attachments (4)

29577.diff (3.4 KB) - added by obenland 6 years ago.
29577.2.diff (763 bytes) - added by kdoran 6 years ago.
29577.3.diff (751 bytes) - added by kdoran 6 years ago.
Converted spaces to tabs
29577.4.diff (1.7 KB) - added by obenland 6 years ago.

Download all attachments as: .zip

Change History (20)

@obenland
6 years ago

#1 @nacin
6 years ago

Looks like 29577.diff truncates html5.js.

#2 @obenland
6 years ago

The patch just served as an example for how to submit a patch in general during an "Intro to contributing to Core" session at WordCamp Los Angeles contributor day.

There should be a valid patch submitted in a few.

@kdoran
6 years ago

@kdoran
6 years ago

Converted spaces to tabs

#3 @kdoran
6 years ago

  • Keywords has-patch added; needs-patch removed

#4 @obenland
6 years ago

  • Description modified (diff)
  • Keywords commit added
  • Milestone changed from Awaiting Review to 4.1

29577.3.diff is ready to go.

Patch created during WordCamp Los Angeles.

This ticket was mentioned in IRC in #wordpress-dev by obenland. View the logs.


6 years ago

#6 @SergeyBiryukov
6 years ago

Does the same apply to _wpCustomizeSettings.values.background_color in Twenty Twelve?

#7 @obenland
6 years ago

  • Keywords commit removed

Yes it does. We have to make sure to test it with 3.5 as well then.

@obenland
6 years ago

#8 @DrewAPicture
6 years ago

  • Owner set to kdoran
  • Status changed from new to assigned

#9 follow-up: @lancewillett
6 years ago

Any updates on this one? Needs more patch? Twenty Twelve ticket and patch also needed?

#10 in reply to: ↑ 9 @kdoran
6 years ago

Replying to lancewillett:

Any updates on this one? Needs more patch? Twenty Twelve ticket and patch also needed?

I think it is good to go Lance. Sounds like Twenty Twelve may need a ticket and patch also. I'm a noob to code contribution. Is there any meaning to the ticket being changed from "New" to "Assigned"?

Kiko

#11 @lancewillett
6 years ago

  • Keywords needs-testing added

If Twenty Twelve needs the change please make a new ticket and add a patch to it.

Is there any meaning to the ticket being changed from "New" to "Assigned"?

I think it usually means that someone is working on it, so it doesn't overlap with another person making a patch.

This ticket was mentioned in Slack in #core-themes by lancewillett. View the logs.


6 years ago

#13 @obenland
6 years ago

  • Focuses javascript added
  • Keywords commit added; needs-testing removed
  • Version changed from 3.6 to 3.5

From Slack:

@obenland: When you have time could you review #29577 please? Need to know if Twenty Twelve also needs a similar patch. And if it's urgent for 4.1 and next theme releases in directory.

29577.4.diff covers Twenty Twelve as well. Just tested it successfully with 3.5.
I'm not sure we really need a separate ticket for Twenty Twelve, we could as well update the description of this ticket to make it broader, if that is the reason.

It's not necessarily urgent, but it's a reasonably small change that has been soaking for all of 4.1, so I don't see a reason to wait either.

#14 @lancewillett
6 years ago

  • Summary changed from Twenty Thirteen: Use proper way to get customizer settings values to Twenty Twelve and Thirteen: Use proper way to get customizer settings values

#15 @lancewillett
6 years ago

I'll update the change at commit time to also bump the JS version numbers where they are enqueued in functions.php files.

#16 @lancewillett
6 years ago

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

In 30482:

Twenty Twelve and Thirteen: Use proper way to get customizer settings values.

Props obenland, kdoran. Fixes #29577.

Note: See TracTickets for help on using tickets.