Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#22461 closed defect (bug) (fixed)

Remove header text 'restore' button left over from Farbtastic

Reported by: drewapicture's profile DrewAPicture Owned by: nacin's profile nacin
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-patch commit
Focuses: Cc:

Description

In custom-header.php, the old Farbtastic color picker relied on a separate button to reset back to the theme default text color. We don't need it anymore.

Patch attached.

Attachments (5)

22461.diff (717 bytes) - added by DrewAPicture 11 years ago.
22461.2.diff (1.1 KB) - added by DrewAPicture 11 years ago.
+ no-js label
22461.3.diff (1.3 KB) - added by lessbloat 11 years ago.
22461.4.diff (1.3 KB) - added by DrewAPicture 11 years ago.
errant space
22461.5.diff (1.6 KB) - added by DrewAPicture 11 years ago.
removes resettext $_POST bits

Download all attachments as: .zip

Change History (23)

@DrewAPicture
11 years ago

#1 @nacin
11 years ago

  • Milestone changed from Awaiting Review to 3.5

#2 follow-up: @helenyhou
11 years ago

  • Keywords commit added

Man, that text was confusing to begin with. I was looking at this patch wondering since when you could restore header text and what it would even restore to, and what restoring it had to do with the color mod.

#3 in reply to: ↑ 2 @SergeyBiryukov
11 years ago

Replying to helenyhou:

Man, that text was confusing to begin with.

Related: #17605

#4 follow-up: @nacin
11 years ago

Could this still be needed for no JS?

#5 in reply to: ↑ 4 @DrewAPicture
11 years ago

Replying to nacin:

Could this still be needed for no JS?

It's my understanding this is in no way connected to the new Iris color picker. That's not to say it couldn't be hooked up to it, but in its current form, it was part of Farbtastic.

Edit: I should add that the no-js fallback should just be a textbox, as it was pre-3.4, so I'm not sure this button would be useful in any current context.

Last edited 11 years ago by DrewAPicture (previous) (diff)

#6 follow-up: @helenyhou
11 years ago

I guess it does help with no JS, since it's an actual form submit, but the fallback to an input is there as well.

#7 in reply to: ↑ 6 ; follow-up: @nacin
11 years ago

Replying to helenyhou:

I guess it does help with no JS, since it's an actual form submit, but the fallback to an input is there as well.

Right, but does the input allow for falling back (resetting) to the default color?

#8 in reply to: ↑ 7 ; follow-up: @helenyhou
11 years ago

  • Keywords commit removed

Replying to nacin:

Right, but does the input allow for falling back (resetting) to the default color?

Nope, unless you knew what it was and enter it yourself. Wonder if we can make a generic fallback for resetting to default. Too late?

#9 in reply to: ↑ 8 @DrewAPicture
11 years ago

Replying to helenyhou:

Nope, unless you knew what it was and enter it yourself. Wonder if we can make a generic fallback for resetting to default. Too late?

What about just falling back to a text input and outputting a label next to it with the default color?

Header Text Color		#[	] Default: #000000
Last edited 11 years ago by DrewAPicture (previous) (diff)

@DrewAPicture
11 years ago

+ no-js label

#10 @DrewAPicture
11 years ago

  • Cc xoodrew@… added

An approach similar to comment:9 is in 22461.2.diff and looks like this:

http://f.cl.ly/items/2Z2H311T3F1E092O3D0a/Screen%20Shot%202012-11-18%20at%203.22.39%20PM.png

Also note the vertical alignments seem generally off in Chrome.

Last edited 11 years ago by DrewAPicture (previous) (diff)

@lessbloat
11 years ago

#11 @lessbloat
11 years ago

In 22461.3.diff​ just moved the default text up next to the field, and added class="description":

http://f.cl.ly/items/3l1D1l193R3s0u062Y41/22461.3.jpg

#12 @DrewAPicture
11 years ago

Looks good to me. Also noticed there's an extra space after the hex code in the printf we can remove.

@DrewAPicture
11 years ago

errant space

#13 @DrewAPicture
11 years ago

22461.4.diff just removed the errant space in the string.

@DrewAPicture
11 years ago

removes resettext $_POST bits

#14 @helenyhou
11 years ago

22461.5.diff is looking good to me. Thanks for remembering to remove the $_POST part :)

#15 @helenyhou
11 years ago

Also, opened #22509 for a generic default color fallback in no-JS.

#16 @nacin
11 years ago

  • Keywords commit added

#17 @nacin
11 years ago

  • Owner set to nacin
  • Status changed from new to accepted

#18 @nacin
11 years ago

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

In 22695:

Custom Header: Remove Farbtastic-era "Reset Text Color" button. Show default color when JS is disabled. props DrewAPicture. fixes #22461.

Note: See TracTickets for help on using tickets.