WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 8 years ago

#16961 closed defect (bug) (worksforme)

Custom Background Color Input/Dialog in Twenty Ten

Reported by: hakre Owned by: jakub.tyrcha
Milestone: Priority: normal
Severity: normal Version: 3.1
Component: Bundled Theme Keywords:
Focuses: Cc:

Description

I'm running into a UI/UX issue when I want to modify the background image and color.

If the background preview is that height that the page needs scrolling, the background color selector does not work any longer.

It's hidden in part and scrolling does not work for scrolling further down because if you want to scroll the element looses the focus and closes, so the scrollbar shortens again.

Attachments (3)

hakre.png (21.8 KB) - added by hakre 10 years ago.
Screenshot of the problem
16961.patch (649 bytes) - added by jakub.tyrcha 9 years ago.
16961.2.patch (906 bytes) - added by jakub.tyrcha 9 years ago.

Download all attachments as: .zip

Change History (12)

@hakre
10 years ago

Screenshot of the problem

#1 @hakre
10 years ago

Screenshot of the problem:

Screenshot of the problem

From my UI coding: elements that have not enough room to bottom / right can move more to the top / left to be available fully on screen.

#2 @hakre
10 years ago

  • Component changed from General to Themes

@jakub.tyrcha
9 years ago

#3 @jakub.tyrcha
9 years ago

  • Cc jakub.tyrcha added
  • Owner set to jakub.tyrcha
  • Status changed from new to accepted

This patch chechs the document height and if the colorpicker would expand it - moves it up

#4 @jakub.tyrcha
9 years ago

  • Keywords has-patch needs-testing added

#5 @nacin
9 years ago

We should be able to improve this a bit:

  • Pass $ to the function, preventing us from using jQuery internally.
  • Don't set up window_height if you don't need it.
  • Initialize colorpicker_offset with var.
  • I imagine this makes it jump/flicker, by showing then moving. Any way we can prevent that?
  • Any way we can also calculate the height of the color wheel, rather than magic numbers?

Just a first glance.

#6 @jakub.tyrcha
9 years ago

Thanks for the input :) As for your remarks:

  • This doesn't seem to work - Chrome does OK with it, but Firefox throws errors
  • I need it, it's used 4 lines later - the reason for getting the document height >before< showing the color picker, is that if the color picker expands lower than the current document, the document height increases and we can't determine how much. IMHO this one should stay this way
  • Good call, thanks
  • No, actually - it doesn't. A normal procedure for positioning an element is unsetting its display value from "none" and setting the position with the same command set. However, commands in JavaScript are not executed one after another but in parallel, so since we do almost no operations between showing the element and setting its position, the delay should be lower than the rendering time (and from my observations, it is)
  • fixed. there is still the +2/+10 thing, but it can't be fixed, because it's just visual styling - we don't want to have the colorpicker fit the input border

how is the new patch?

#7 @SergeyBiryukov
8 years ago

  • Component changed from Themes to Appearance

#8 @SergeyBiryukov
8 years ago

  • Component changed from Appearance to Bundled Theme

Actually, the old color picker is specific to Twenty Ten and Twenty Eleven.

#9 @lancewillett
8 years ago

  • Keywords has-patch needs-testing removed
  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from accepted to closed

Fixed in 3.5 with the change from farbtastic to Iris color picker, see r22030.

Note: See TracTickets for help on using tickets.