#22058 closed enhancement (fixed)
Improve custom background properties UI
Reported by: | grapplerulrich | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 3.4.2 |
Component: | Customize | Keywords: | has-patch has-screenshots |
Focuses: | ui, accessibility, administration | Cc: |
Description
For the the custom background display options there is only left, centre and right position. The top and bottom position is missing.
Attachments (35)
Change History (151)
#3
@
12 years ago
- Keywords 2nd-opinion added
I created a solution. I attached the two files that I edited.
Here is the test site I am running on.
http://www.grappler.tk/wpdev/
This is my first edit so please bear with me.
#4
@
12 years ago
- Keywords needs-patch added
Rather than full files, you can submit a patch using Subversion:
http://make.wordpress.org/core/handbook/submitting-a-patch/
#5
@
12 years ago
I have changed the files to patches.
Here is a screenshot of the admin view.
http://pix.am/LSq0/
I tested in in IE7-9, Opera, FF and Chrome on Win7 and it works.
#8
@
12 years ago
I have added the patch with the customizer. Here is a screenshot.
http://pix.am/dPVz/
#9
follow-up:
↓ 10
@
12 years ago
@grapplerurlrich the patch works if you set horizontal then the vertical but if you go back to change the horizontal after setting vertical it sets vertical to top. At least in the preview, it does save them correctly and display as expected on the site.
#10
in reply to:
↑ 9
@
12 years ago
Replying to MikeHansenMe:
@grapplerurlrich the patch works if you set horizontal then the vertical but if you go back to change the horizontal after setting vertical it sets vertical to top. At least in the preview, it does save them correctly and display as expected on the site.
What tool do you use to minifiy the js?
#11
@
12 years ago
Set define( 'SCRIPT_DEBUG', true); then edit the dev.js file instead of the min file.
#12
follow-up:
↓ 13
@
12 years ago
@MikeHansenMe Thanks, I updated the patch. I also saw a problem with the "background-attachment:" and fixed that too. How long will it take for it to be implemented in WordPress?
#13
in reply to:
↑ 12
@
12 years ago
Replying to grapplerulrich:
@MikeHansenMe Thanks, I updated the patch. I also saw a problem with the "background-attachment:" and fixed that too. How long will it take for it to be implemented in WordPress?
Tested and can confirm the customizer is now working as expected. We are currently in a feature freeze for this release cycle so it will likely be reviewed after 3.5 comes out when enhancements are being considered. Thanks for contributing.
#14
@
12 years ago
Related: ticket:14979.
#15
follow-up:
↓ 16
@
12 years ago
Patch still applies but tested again and the changing the vertical position to center shows no change. It does save the change but does not show the change immediately as the horizontal change does.
#16
in reply to:
↑ 15
@
12 years ago
Replying to MikeHansenMe:
Patch still applies but tested again and the changing the vertical position to center shows no change. It does save the change but does not show the change immediately as the horizontal change does.
Please check: attachment:22058.diff. I've refreshed the patch and filled out missing parts.
#17
follow-up:
↓ 18
@
12 years ago
Patch does not seem to show the change when center or bottom are selected.
#18
in reply to:
↑ 17
@
12 years ago
Replying to MikeHansenMe:
Patch does not seem to show the change when center or bottom are selected.
I'm testing it in Firefox 16 with Twenty Eleven and it works as intended, both from Theme Customizer and Appearance > Background.
#19
follow-up:
↓ 20
@
12 years ago
When you change the vertical positioning of the background image the preview does not update. It does after saving and refreshing. However, it should happen like horizontal does with immediate preview before saving. I tested using FF 20 and Chrome 25 on Ubuntu. I can test on Windows tonight to see if it works on Windows w/ IE.
#20
in reply to:
↑ 19
@
12 years ago
- Keywords needs-testing added
Replying to MikeHansenMe:
When you change the vertical positioning of the background image the preview does not update. It does after saving and refreshing.
Are you sure you're testing the latest patch? Please revert any changes and then apply attachment:22058.diff. Just tested it on IE and it seems to behave correctly. Where exactly do you encounter the issue, in Theme Customizer, in Appearance > Background or both? What are the steps to repoduce it? Thanks for helping.
#21
follow-up:
↓ 22
@
12 years ago
I am running the most current trunk version(3.6-beta1-24023) and reverting all patches prior to testing. Then applying your patch 22058.diff
Please see the video for explanation
http://bit.ly/11CSUBC
#22
in reply to:
↑ 21
@
12 years ago
Replying to MikeHansenMe:
Please see the video for explanation
http://bit.ly/11CSUBC
Well, that's really strange. Just tested it again, now using IE9 and FF21 on Windows and it works as expected. Please check that you have defined SCRIPT_DEBUG
in your wp-config.php
file:
define('WP_DEBUG', true); define('SCRIPT_DEBUG', true);
It seems that everything is loaded, except recent changes to JS. Maybe you're using the minified scripts which are not affected by the patch. If it's not this, please take a look at the console and let me know if there are any errors. Thanks!
#23
follow-up:
↓ 24
@
12 years ago
I'm seeing the same thing as MikeHansenMe in Chrome 25.0.1364.160.
... and yes, I'm using SCRIPT_DEBUG.
#24
in reply to:
↑ 23
@
12 years ago
Replying to bpetty:
I'm seeing the same thing as MikeHansenMe in Chrome 25.0.1364.160.
... and yes, I'm using SCRIPT_DEBUG.
After applying the patch go to Appearance > Background (or Customize). Then press Ctrl + F5 to reload (override cache) and try uploading an image again. Please let me know if it helps.
#25
@
11 years ago
- Cc catalin.dogaru@… added
- Keywords ui-feedback ux-feedback added
Current patch still applies but I'm not really satisfied with the growing list of radio buttons. I've attached two concepts illustrating variations of the idea that could improve the interface and usability.
attachment:22058-editor.png puts accent not only on functionality but on UI improvements too.
attachment:22058-presets.png simplifies the idea even more exposing only common background settings presets. It's not as flexible as the first solution but it's much easier to use.
What do you think and which one do you prefer? Would it improve the existing UI/UX?
#26
follow-up:
↓ 27
@
10 years ago
- Focuses ui administration added
- Keywords 2nd-opinion has-patch needs-testing ui-feedback ux-feedback removed
I really like the idea of an alternative UI to the radio buttons - the position concept in particular is very compelling. The others seem a little harder to understand as plain icons when you're not versed in CSS - even the words are tricky. For instance, attachment seems like it could be a checkbox, for instance, and called "Scroll with page" or something.
#27
in reply to:
↑ 26
@
10 years ago
helen, great! What do you think of attachment:22058-background.png?
This ticket was mentioned in Slack in #core by cdog. View the logs.
10 years ago
#29
@
8 years ago
- Keywords has-patch ui-feedback needs-testing added
- Milestone changed from Awaiting Review to Future Release
- Summary changed from Custom background vertical position to Improve custom background properties UI
22058.1.diff builds on all of the previous work here to introduce an almost-entirely-reworked UI for custom backgrounds in the customizer.
Text radio buttons are replaced with visual representations of each option, background positions can be set vertically and horizontally, background-size can be set (merged from #26386), tiling is depicted visually, and background attachment is represented with a "scroll imge with page" checkbox.
This should all be backwards compatible, although the background attachment checkbox could use a bit more work. I kept the existing work on the old backgrounds admin page in the patch, but didn't add background-size there since that page is only shown to no-JS and IE7 users by default, and isn't really maintained or supported by most themes at this point.
The UI could use refinement - feels fairly heavy to me, which could probably be resolved with color, but I want to ensure that there are nice big tap-targets, consistency with the use of a single icon when representing each of the options visually, and using white backgrounds on the buttons to indicate clickability. The buttons-as-tiles follows a similar UI to the media modal, with the use of thick borders to indicate selection and hover. Definitely want to avoid making these look like the core "buttons" style, because there are so many of them and that would add a lot of visual clutter (with rounded borders, shadows, etc). The hope is for them to read as images representing each choice. See the screenshots and gif above for the visual. Because traditional radio inputs can't be used here, I went with <button>
s for each choice, with screen reader text representing each option with the hope that this can be similarly accessible.
In the background, there's one new customizer control to handle all of the image-select controls. This could potentially be used outside of core for other things, but it requires quite a bit of CSS specific to each use and one of the background options needs to handle two settings assigned to one control, so it's name is specific to backgrounds for now. It would be cool to build something like a layout control that uses a similar UI in core for themes and plugins to extend in the future, but let's focus on this one control for now.
It's been a long time since custom background has gotten much attention, so let's polish this up in the next few weeks so it's ready early for 4.7.
This ticket was mentioned in Slack in #design by celloexpressions. View the logs.
8 years ago
#33
@
8 years ago
I find this a bit complex. It would certainly put me off using the function or recommending it to clients. [cdog's version from 17 months ago] is a bit better but the functionality is fairly obscure. The results look a little MySpacey. Are there people using these background functions and are they using the edge cases? While the current flexibility is amazing, it would be far clearer to have just three simple, attractive and usable presets available in the standard interface.
#34
@
8 years ago
- Keywords has-screenshots added
22058.2.diff tweaks the colors to match the text color (#555
) for the icons and use the darker gray for the selected border, and this feels better to me. It also fixes a bug on screens between 400-600px wide with the background position control.
@afercia: is the use of <button>
s with screen reader text contents for each choice an appropriate way to handle accessibility? I tested on keyboard and it works well there, not sure about screen readers though.
@FolioVision: while I'd prefer not to have these options at all, this is how the custom background functionality works. If it were being built today, I doubt we'd have user-facing options for anything other than the image. But given that this feature was originally introduced 5+ years ago, we need to work with the assumptions that it makes. I don't know that we could determine three reasonable pre-set combinations that would be appropriate in the context of a given theme or image, let alone for core in general. I would also guess that a majority of themes no longer use this feature and a majority of users don't mess with the options, perhaps beyond setting an image.
It's really hard to make many assumptions about what a user might want to do with these options besides the fixed/cover-sized approach that's probably the most common right now. We know that background-repeat doesn't apply when background-size is cover, but beyond that we can't really hide any options based on other settings. A lot depends on the size and aspect ratio of the selected image and the size and orientation of the end user's screen. My goals here are to make the options more visual, reducing the use of technical terminology, and to fill in the clear gaps in functionality while prioritizing the most important options and providing a more appropriate flow between options. That improves the experience for the users that do use this feature while maintaining full backwards compatibility.
#35
@
8 years ago
@celloexpressions
Point well made about not removing functionality, Nick. I played around with the existing background image interface today, and was pretty happy with how simple and smooth the action is. The only functionality which seems missing is the option to compress a large image to fit within the background of a smaller mobile device (although with hires mobile style screens becoming more common, this issue could solve itself: Retina type screens could just use the desktop settings on a per pixel basis which effectively is hires).
There is an argument to be made for making the options visible (but greyed out) before someone uploads an image. It took me a few minutes to understand why there are no options just an upload button. The counter argument is that the additional options even greyed out creates additional mental strain for a normal user analysing information presented about an unused feature. I can see both sides here.
If we agree that background image is not best practices to build a website these days, it seems counter productive to add a whole lot more edge case options - additional options suggests to me that we encourage people to use this tool and those options.
The long run danger I'm worry about would be to make customizer unwieldy, rather than making customizer a cheap and cheerful way to customize a standard theme. The best version of Microsoft Word, it is said, is version 5.1 for Macintosh, released in 1992. There were enough features, elegantly put together to allow a user to learn the program quickly and intuitively and produce very high calibre documents. Almost everything added since have been excuses to force an upgrade. As we aren't selling WordPress (free), in theory we should be exempt from features for features sake.
#36
@
8 years ago
Background size is really the only new option here, which as you mention is necessary for most cases. The vertical position (which this ticket was originally for, see comments from 3-4 years ago) is more complementing the existing position option and integrates into the same control choice. I think maintining the existing hidden behavior is a good way to keep this UI from making the customizer feel cluttered, as you'll never see it unless you upload an image, and if it looks good after setting the first option (size) or two, you probably won't even look at the more complex things like position.
#37
@
8 years ago
I will leave more in-depth design considerations to the UI/design team, just wanted to say that I feel this would add a bit too clutter to the UI maybe for little benefit. Comparing the current UI and the proposed one side by side, the latter gives me the impression of a wall of at-first-sight-pretty-obscure icons vs the simplicity of the former:
About accessibility and semantics, radio buttons would be the best option because that's what they are meant for: a single choice between related options. By the way, they would need to be grouped in a fieldset with a legend. About the importance of fieldsets and legends I'd recommend this post by Leonie Watson, goes directly to the point with simple examples: https://accessibility.blog.gov.uk/2016/07/22/using-the-fieldset-and-legend-elements/
Worth mentioning there are various CSS techniques to hide the radio buttons and use their label elements to provide a different visual, I think Press This uses one of them.
Please consider every time there's the intent of changing and transforming native controls, then there's the need to replicate all their native functionalities otherwise the native level of accessibility will be decreased. I understand the point to make the options more visual, reducing the use of technical terminology but this shouldn't happen at the cost of reduced accessibility. My general recommendation would be to keep the current radio buttons and just improve the labels wording.
Additional considerations:
- icon-only controls are tricky and ideally icons would need a text label
- whether they're going to be radio buttons or buttons, they need to be grouped in fieldsets with a legend
- proper fieldsets and legends would also allow to simplify the labels: no need to repeat "Set Tile Background to...", "Set Tile Background to...", "Set Tile Background to..." etc. for each option
- selected state: if radio buttons, that's natively announced by assistive technologies but if
<buttons>
then they would need anaria-pressed
attribute, as done for the "device preview" buttons - initial state (edit: I meant initial default value): not clear why some properties have a clear indication of the initial value and others don't, see also the
false
value set for these properties in the screenshot below - technical terminology: completely agree to avoid it, I'd also add that some terms are difficult to translate, for example "tile" and maybe the wording should just explain users what a control does with the simplest possible wording
#38
@
8 years ago
The proposed UI is really heavy and hard to understand. It's honestly super overwhelming. The repetition of the image icon and overall lack of hierarchy makes it super confusing to parse. It's hard to figure out what to look at first when it's just the same thing over and over again.
https://core.trac.wordpress.org/ticket/22058#comment:26 is a much easier to understand approach, IMO. I'm also attaching what WordPress.com does, for comparison.
#39
@
8 years ago
Firstly, I think a lot of the issues with this solution come from not creating one from observing and testing real users. Note I say real because we are all not normal in our use and creating any solution just because we think it's a good idea without observing, testing and then creating from that point, it isn't a good idea. If we identify a possible issue, we should gather data, observe and then come up with a solution. Not cobble together a best intention. I don't doubt that what has been created is meant to be that and has great intentions at its heart. Unfortunately, it falls short of usability.
What we seem to have after a lot of input and various voices is something very over-engineered for the solution. Something this complex we should always stop carrying on the complexity and consider where we went wrong. I'd say the wrong point here is not actually creating after observing or looking for designer input earlier. That later may have just been a case of us not having enough to spread to Customizer, we're are all try and fix that though and slowly have a better team.
This all said, user testing and actually finding out what does and doesn't work for users isn't something only designers can do. I at no point unless mistaken see this happening for this enhancement. This has got over time more and more complex and more and more iterations without ever having user tested. That's something we should stop as a behaviour. I know we don't always have the people around, but this really is important. I'm happy to help anyone find out how they can do user testing - it shouldn't be down to just a few people or just designers.
This UI to me is a classic example of a paradox of choice, your eyes can't settle and no clear path is obvious. That's bad and not going to ensure it works. I actually beyond that have a lot of issues with even knowing where the positions are in these images. I do not feel this at all is the right path.
I don't think the intention of this ticket is bad and I actually think with some research we could come up with a solid solution. I do not feel the current one on table is that. @melchoyce shows how .com does it and I think that's worth adding as a simpler but just as effective solution. Do we have to do that? No. But, lets actually pause and find the point we escalated without user testing and iterating through a user focused design process.
I am commenting late in this tickets life and I know that can be horrible to do. I don't meant to drive by comment, but we are trying as said earlier to give the attention Customizer tickets deserve from the design team. Please drop tickets like this into the design weekly chat - I'm fairly sure I can speak for everyone that comes to that chat and say we'd love that and promise to take action. I'm also going to spend a few hours tomorrow and look over any flagged for Customizer UI or UX, hopefully we can ensure we don't have to come in late on other features, but we can work together.
#40
@
8 years ago
Thanks for showing us the .com interface for this feature, Mel. It's considerably lighter. Still I'm not certain it's better than the bare bones text version we have now, apart from the handy colour picker.
How do other people feel? Is the .com version or the text version better for you?
#42
@
8 years ago
I will note that the proposal I posted above was based on a problem identified by observing inexperienced users, via my work at the USC Annenberg Digital Lounge, and the proposed UI is inspired by other projects we've completed in that context for these very non-technical users.
However, based on the discussion above, there are a lot of problems with moving toward a more visual interface. WordPress.com uses icons now, but they present the same challenges as the version I posted, with graphical representations that may or may not be clearer for users, and resulting accessibility challenges (regardless of the actual markup used). We could also nitpick the exact UI - there are other dashicons that would perhaps work better, and I would be concerned about touch accessibility - but despite being considerably different visually, the UI structure is the same and has the same problems. Namely, the functionality is ambiguous but not necessarily worse than the current technical text labels.
Taking a step back, my goal for this ticket is to find a more usable balance between the background property options and an ideal user experience. After some thought, I'm increasingly thinking that the theme should be responsible for all background properties, with only the image being customizable through UI in the customizer. In the rare cases that something else is needed, it can be accomplished with custom CSS via #35395.
Rather than forcing users to decide, theme should set default background-position, background-size, background-repeat, and background-attachment values in their CSS based on the intended use of backgrounds in the theme. If a theme is designed for a tiled texture background, it probably wouldn't make sense for a user to replace this with a full-screen fixed photograph, and vice-versa. This approach would also improve the way custom backgrounds are used with respect to themes, and but the decisions back on theme designers, where they should be. Right now, many themes seem to add custom background support for the sake of it, without considering whether a custom background will look good with the rest of the theme design.
As @FolioVision and I discussed earlier in this ticket, the path to removing these options may be difficult, but if we really want to we could try to make it happen.
This ticket was mentioned in Slack in #design by hugobaeta. View the logs.
8 years ago
#44
follow-ups:
↓ 45
↓ 62
@
8 years ago
Talked through this a bit in design chat, the proposals and examples have been really helpful in figuring out what actually needs graphical representation and what doesn't. Here's what I'm thinking:
- Image control
- Background size: radio or dropdown with
Original
,Fit to screen
,Fill screen
. - Repeat: checkbox, checked by default.
- Scroll with page: checkbox, checked by default.
- Start position: graphical helper, such as in 22058-background.png.
Note that background size is new and repeat omits vertical and horizontal repeat - they are certainly used sometimes, but even in my experience as a developer, very infrequently for general body backgrounds. It could also be called "Tile", perhaps, but I find that a little less descriptive when standing alone.
I also like the presets in 22058-presets.png but I have a feeling it would be extremely contentious to further reduce options like that and may not be worth it. For reference though, it's rather like what you find in OS background options:
#45
in reply to:
↑ 44
@
8 years ago
Replying to helen:
- Start position: graphical helper, such as in 22058-background.png.
I also like the presets in 22058-presets.png but I have a feeling it would be extremely contentious to further reduce options like that and may not be worth it. For reference though, it's rather like what you find in OS background options:
What about something very simple for position like 9 circular radio style buttons (like a tick tack toe grid) for position? In term of the Fill, Fit and Stretch options, perhaps its better to leave them text as in your visual example.
Here's an example of the simplicity of that kind of a grid.
And here's what it would look like in context (current design referenced by @celloexpressions and @afercia above).
#46
follow-up:
↓ 47
@
8 years ago
@FolioVision: I like this!
Don't beat me, but what about adding cover & contain?
#47
in reply to:
↑ 46
@
8 years ago
Replying to Presskopp:
Don't beat me, but what about adding cover & contain?
Please read my comment above - background size is covered.
#48
@
8 years ago
- Keywords needs-patch needs-screenshots added; has-patch needs-testing has-screenshots removed
- Milestone changed from Future Release to 4.7
I like the simplicity of the nine radio dots, but I'm not sure whether or not users could understand what it means. It could work, but should definitely have at least one inexperienced user test it.
I like the five (ordered) options outlined in comment:44. However, I think that repeat/tile should be off by default. In my testing, I noticed significant performance issue with certain combinations of properties (I think particularly relative to background size and with larger images, in Chrome) when repeat was on, so it would probably be safer to have that off until specifically desired. We can also hide that option when background size is cover
.
I can work on a revised patch and screenshots to reflect that late next week, unless someone beats me to it.
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
#50
@
8 years ago
Ladies and gentlemen: we're happy to help with the CSS code for the 9 point position picker (or other issues) as soon as the final requirements settle.
@Helen, taking my visual from comment 45 what other additions need to be made? What order would you like the properties in? I know you covered this material once comment 44 but before doing a final mockup, I'd like to confirm your thoughts on repeat (which as celloexpressions notes can cause huge performance issues).
To be honest, I think we know what combinations can kill a site loading and we should show errors (even permanently inline on this dialogue box) warning that that the users choices will make for a very slow loading website. If it were entirely up to me, I'd make it impossible to make those choices at all in the GUI customizer. The internet would be a faster place if anyone who wants to bork their site must learn to code by hand!
#51
@
8 years ago
I am a little concerned using something so well known as a radio button UI for something like this. It's taking something that whilst in the 9 grid isn't a radio, well it is still. That brings a lot of user confusion potentially. I would caution this as an approach. In saying that, if someone can show me user tests or is interested, we may have fears proven or laid to rest.
#52
@
8 years ago
@karmatosed Thanks for sharing your concerns. The simple array of nine is so much clearer than anything else on the table, perhaps we should move ahead with the improvement so this ticket doesn't languish in another four years of Trac limbo (really it's been four years).For me the button by default showing in the middle suggests position. Tic tac toe has been around longer than radio buttons.
That said, for differentiation, the round radio buttons could be turned square or the existing round ones could be put in a square box to suggest a page. Anything more and we risk building an ugly interface. Thoughts on square buttons or a square around the nine buttons?
I'd love to help finish this up but that means decisions on all sides.
My post above posed some questions about performance which really worry me. Does anybody have any thoughts on offering users performance warnings here?
#53
@
8 years ago
I don't agree we should rush in without user testing. Sometimes tickets take a while and perhaps what seems clearer to use isn't. For what it's worth, I don't see the 9 as the better solution, as I've said. I am however keen we get user feedback on this.
This ticket was mentioned in Slack in #design by karmatosed. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
This ticket was mentioned in Slack in #design by celloexpressions. View the logs.
8 years ago
#57
@
8 years ago
Summarizing the discussion in the design chat, it sounds like we don't need to add the vertical-position options due to lack of need/support requests for them (or background-related options in general). We did decided to go with the approach from @cdog if we do add those though. Therefore, we need an updated patch that implements 43.
We also decided to explore an alternate approach that removes these options in favor of a preset-selector (as has been suggested before), which would internally set the values of the existing options for backwards compatibility. We'll need mockups and a patch for that approach as well.
#58
@
8 years ago
Hey there! Here's a new approach based on what was discussed so far: 22058-customize-background.png.
I've merged my previous proposals (22058-background.png and 22058-presets.png) to a single unified interface, keeping both built-in presets and configurable options.
The presets would be:
- Fill Screen
- Fit to Screen
- Stretch to Screen
- Center
- Repeat
- Custom
They don't require a preview image anymore as background options are now available inside the customizer only (the wording itself is self-explanatory and you also get a live preview for your choices). I've moved them to a dropdown menu.
Selecting a preset would change the options below based on your selection. Tweaking the options would select the Custom preset automatically.
The background position component may seem a bit too big. I've enlarged the buttons to make them easier to tap on touch devices as they don't have any spacing between them.
This could be achieved using existing dashicons and without creating any new assets. Next, I'll provide a working patch for a potential user test. What do you think? Any feedback is much appreciated.
#59
@
8 years ago
I like it. There's a bit more 3D shadow in the background position 9 point graphic than currently customary but otherwise it's nice and simple and doesn't encourage performance killing choices (although on second thought some of them are available). Perhaps we should shorten the list of background presets.
#60
@
8 years ago
A side by side comparision (22058-compare.png) between the current UI and the proposed one.
Here I also added back the options for background repeat:
- No Repeat
- Repeat Horizontally
- Repeat Vetically
- Both
I'm not yet convinced if this should be limited to repeat/no repeat only. See 22058-background-patterns.png for some common repeat patterns. While the second scenario may not be used as much today (fixed-width content with vertically repeated background), the first one may still apply in many cases (top aligned horizontally repeated background).
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
#62
in reply to:
↑ 44
@
8 years ago
Anyone up for a test? 22058.3.diff is 44 with the UI from 22058-customize-background.png.
Where to look after applying the patch:
- Dashboard
wp-admin/themes.php?page=custom-background - Customizer
wp-admin/customize.php?autofocus[control]=background_image
The patch removes repeat-x
and repeat-y
options from the interface but they are still supported to preserve backward compatibility.
If you set them explicitly they still work. From theme:
<?php add_theme_support( 'custom-background', apply_filters( 'twentyfifteen_custom_background_args', array( 'default-repeat' => 'repeat-x', … ) ) );
Or WP-CLI:
$ wp theme mod set background_repeat repeat-x
To test default settings:
$ wp theme mod remove background_size background_position_x background_position_y background_repeat background_attachment
This could be further improved by adding predefined presets which set the background options automatically and toggle the visibility of controls based on user choices. Looking forward for some input. Thanks!
This ticket was mentioned in Slack in #core by cdog. View the logs.
8 years ago
#64
@
8 years ago
Some clarifications on how presets would work with the actual patch. Here's what I'm thinking:
- The preset slector should be the first item, above all other options;
- Choosing a preset would change the options below automatically;
- Manually editing any options would change the preset to Custom.
Ideally a user workflow would be to upload a background image and choose a preset, then Save. Rest of the controls would be useful for further tweaking the background settings (if needed at all) or if there isn't any preset matching the user's preferences.
I've uploaded a screenshot of Customize > Colors (22058-color-presets.png) to make an analogy. A user may use a color scheme as it is, customize it, or select each color option to create something new (in this order, I think).
22058.3.png is a screenshot of the patch (22058.3.diff) as it is now. The presets have to be added and for this it would be great a review for what has been done so far.
This ticket was mentioned in Slack in #design by hugobaeta. View the logs.
8 years ago
#66
follow-up:
↓ 67
@
8 years ago
- Keywords has-patch needs-refresh has-screenshots added; has-ui-feedback needs-patch needs-screenshots removed
Great work here @cdog, I reviewed the patch and didn't see any major issues. A couple of suggested changes:
- Require and register the position control at the top of each corresponding block of customize controls, since it's most closely related to the base control (and we shouldn't separate the site icon control from the cropped image control that it extends).
- Omit the
settings
parameter when adding thebackground_repeat
control, since the setting id matches the control id and will be set automatically.
It looks like the new file for the background position control was missed in the patch; could you try remaking the patch file to include that? Everyone testing will get a fatal error without that.
#67
in reply to:
↑ 66
@
8 years ago
Replying to celloexpressions:
Oh snap! I'm on it, thanks for review!
#68
@
8 years ago
@cdog, these new mockups look great. I think the idea of a compact 9 square position selector is nice and simple.
However, I'm concerned about the arrows going out from the center. I showed it to a few non-technical users and several thought that it was controlling the stretching/sizing of the background, despite the labels.
#69
@
8 years ago
@celloexpressions: I've refreshed the patch and added the missing file, see 22058.4.diff. It seems that you put some effort into your patch/mockups too so thanks for letting me take the "lead", what you do is really helpful :)
@Kelderic: Maybe changing the wording from "Position" to "Alignment" would help? Or trying out alternate icons? Here's the Align extended toolbar from Open Office. Photoshop/Illustrator does use similar icons for object alignment. Altough I find arrows easier to understand.
#70
follow-up:
↓ 73
@
8 years ago
Personally I think that having the squares in addition to the arrows makes it easier to understand, but at the same time I am worried about overly complex icons.
What we really should do is come up with a couple options, say just arrows, then arrows with squares, then maybe just radio buttons inside the boxes, and run user tests.
#71
follow-up:
↓ 72
@
8 years ago
Can I suggest that the Background Scroll default to scroll? And/or warn users that fixed background images don't work as expected on mobile devices using viewports?
It's a bit confusing to me to see the screen icons at the bottom of the customizer. Do these settings apply only to the type of screen currently selected? So I have to set it 3 times? or something like that? If so, maybe a different default for the mobile view.
#72
in reply to:
↑ 71
@
8 years ago
@joyously: Background scroll defaults to scroll
in core (https://core.trac.wordpress.org/browser/trunk/src/wp-includes/theme.php#L1694) and this patch doesn't change it. It's Twenty Fifteen who overwrites it to fixed
(https://core.trac.wordpress.org/browser/trunk/src/wp-content/themes/twentyfifteen/functions.php#L123). Maybe we should suggest it for Twenty Seventeen not to add the same overwrite, but themes in general should be able to define whatever defaults works best for them.
Regarding notifications, why not? Let's see where #35210 is going.
The screen icons at the bottom adjust the live preview viewport size and are hidden on small screen sizes. They are part of the customizer being present on all sections, not only background image. If you have some concerns about their functionality please fill in a separate ticket, I find it broader than the current topic.
Thanks for getting involved!
#73
in reply to:
↑ 70
@
8 years ago
@Kelderic can you provide some mockups? Then we can adjust the patch and prepare several user tests if this is working.
#74
@
8 years ago
Replying to cdog:
@Kelderic can you provide some mockups? Then we can adjust the patch and prepare several user tests if this is working.
Here are some mockups of different styles. I think breaking it up into two controls might be the easiest, but I also really like the idea of having a 4x4 grid, with a 2x2 image icon (see last image).
#75
@
8 years ago
This is starting to feel like we're going overboard again. What happened to the suggestion to get rid of all 9 possible positions and limit to left/right/center? (Mentioned in #57)
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
#78
@
8 years ago
- Owner set to cdog
- Status changed from new to assigned
@cdog can you update the patch to remove the vertical position options and instead show a 3-button group with left, center and right? We should also use the image-align icons used in the editor. If you could also add the proposed presets that would be great.
I'll try to help make sure this makes 4.7, I think we're really close.
#79
@
8 years ago
Before updating the patch I've added two new images.
@celloexpressions Any reasons not to keep the vertical position options? The background-position property was defined in CSS Level 1 and is fully supported even by legacy browsers like older versions of IE. Removing it was suggested before, although this is how the ticket started. Can someone elaborate on this?
@melchoyce, @jorbin I completely agree with you. How to simplify, do you have any suggestions?
22058-variations.png shows three variations based on the feedback received so far. Can we pick one of them to go with the patch or there is room for more improvements? Personally I'd stick with option 3. @helen what's your opinion on this?
22058-options.png shows a possible implementation (similar with the Menus section from Customizer) of how we can toggle the background options visibility. This should work with any of the above variations. Hiding all options would leave visible only the image and presets.
Again, any feedback on this is kindly appreciated. Thank you!
This ticket was mentioned in Slack in #core by cdog. View the logs.
8 years ago
#81
follow-up:
↓ 82
@
8 years ago
Thanks, @cdog.
Rather than adding screen options, we should focus on having as few options as possible, and only showing them when they're relevant. The presets vs. custom settings would take care of that.
Can you add a list of the proposed presets? We can discuss/vote on those and the three design variations above in tomorrow's design meeting.
#82
in reply to:
↑ 81
@
8 years ago
You're right, here's what I'm thinking. I'm changing a bit the behavior from the images (22058-variations.png).
The presets would be:
- Fit to Screen (no options visible as position, size, repeat and scroll doesn't make too much sense to change);
- Fill Screen (same as Fit to Screen);
- Repeat (only position and scroll visible);
- Custom (all options visible).
In this case there's no need to add the screen options (22058-options.png).
Let me know tomorrow after the design meeting how it went. And a decision on position control would be great (horizontal alignment only or both horizontal/vertical). Thanks!
This ticket was mentioned in Slack in #design by karmatosed. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by desrosj. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by celloexpressions. View the logs.
8 years ago
#89
@
8 years ago
22058.5.diff is almost done. @celloexpressions can you take a look?
Please decide on presets (default values and visible controls). See: 22058.5.gif. The Default preset reverts the settings to core/theme defaults.
This ticket was mentioned in Slack in #design by celloexpressions. View the logs.
8 years ago
#91
follow-up:
↓ 92
@
8 years ago
Looking pretty good @cdog. I think we should call the new control WP_Customize_Background_Position_Control
, as it isn't quite a generic positioning control, but could still be useful in themes and plugins. Can we avoid the repetition in there? Maybe loop through a keyed array of values => labels for the nine elements?
#92
in reply to:
↑ 91
@
8 years ago
Thanks a lot for your support, @celloexpressions! I'll do a code cleanup/refactor today (responsive styles needs some work too). Anything else, are the presets agreed?
#93
@
8 years ago
Yes, the options and UI were approved in the design meeting this week based on 22058.5.gif. Thanks for the handy visualization @cdog! Can you get a finalized patch up for final review/commit?
#94
@
8 years ago
Final patch: 22058.6.diff.
@celloexpressions This should be ready. Please double-check the visibility of controls and default values for each preset (and let me know if there is anything to be updated). Thanks!
var visibility = { // position, size, repeat, attachment 'default': [ false, false, false, false ], 'fill': [ true, false, false, false ], 'fit': [ true, false, true, false ], 'repeat': [ true, false, false, true ], 'custom': [ true, true, true, true ], };
var values = { // position_x, position_y, size, repeat, attachment 'default': defaultValues, 'fill': [ 'left', 'top', 'cover', 'no-repeat', 'fixed' ], 'fit': [ 'left', 'top', 'contain', 'no-repeat', 'fixed' ], 'repeat': [ 'left', 'top', 'auto', 'repeat', 'scroll' ], };
#95
@
8 years ago
The multi-select control isn't keyboard-accessible currently. I think adding tabindex="0"
to the input element, and then providing a visual focus style for each radio input styles as a grouped button there that matches the hover style would probably be best. @afercia can you advise whether the implementation in the patch works otherwise for accessibility?
Everything else in the patch looks ready to commit.
#96
@
8 years ago
22058.7.diff fixes keyboard accessibility. @celloexpressions, @afercia can you test the latest patch?
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
#99
follow-up:
↓ 100
@
8 years ago
Tested the latest patch and discussed in today's accessibility bug scrub, we'd recommend a couple improvements. Screenshot first:
The "dot" you see in the VoiceOver panel is announced as "bullet". I think it's the icon, so I'd suggest to use aria-hidden="true"
on the span that holds the icon.
A nice improvement, if possible, would be wrapping the 9 input radio buttons inside a <fieldset>
element with a <legend>
. Basically the text "Image Position" should be the legend text. Not sure if it can be done right now or if it's more tied to the Customizer controls. Alternatively, a legend hidden with screen-reader-text
could work. Worth noting the text "Image Position" is just currently inside a <span>
so the semantics could be improved a bit.
#100
in reply to:
↑ 99
@
8 years ago
@afercia thanks for the input! Please check 22058.8.diff for accessibility improvements. For other controls this should be addressed as a separate issue I think.
Where to look after applying the patch:
- Dashboard
wp-admin/themes.php?page=custom-background - Customizer
wp-admin/customize.php?autofocus[control]=background_image
#101
@
8 years ago
- Keywords commit added; needs-refresh removed
- Owner changed from cdog to westonruter
- Status changed from assigned to reviewing
This might need a few minor technical tweaks but generally looks ready for commit review @westonruter.
#102
follow-up:
↓ 103
@
8 years ago
@cdog looks good to me. Minor detail, out of curiosity why the CSS class ui-helper-hidden-accessible
? Seems to me it's the same as screen-reader-text
and if jQuery UI is used, it should be overridden by the core one anyways.
#103
in reply to:
↑ 102
@
8 years ago
@afercia ui-helper-hidden-accessible
is an alias of screen-reader-text
. See: wp-admin/css/common.css. We can switch with the later if it's causing any trouble (it should have the same effect). I haven't used the display
or visibility
properties to hide the radios, as hidden inputs don't receive focus (i.e. pressing tab skips them).
Also forgot to add a comment for some CSS lines (for explanation). In wp-admin/css/themes.css
I've added:
.background-position-control .button-group .button { -webkit-border-radius: 0; border-radius: 0; -webkit-box-shadow: none; box-shadow: none; height: 40px !important; line-height: 37px !important; margin: 0 -1px 0 0 !important; padding: 0 10px 1px !important; position: relative; }
The properties with !important
get overridden by core responsive styles (see: wp-includes/css/buttons.css).
Thakns again everyone for getting involved!
#104
follow-up:
↓ 105
@
8 years ago
@cdog I know it's an alias :) I mean, ui-helper-hidden-accessible
could be dropped in the future, screen-reader-text
is here to stay 😉
#105
in reply to:
↑ 104
@
8 years ago
@afercia got it, fixed in 22058.9.diff. Also added the missing comment.
#106
@
8 years ago
Code review and revisions being done at https://github.com/xwp/wordpress-develop/pull/184
#107
@
8 years ago
@cdog please review my changes: https://github.com/xwp/wordpress-develop/pull/184/files/82db6ff..cf2fdb2
#108
@
8 years ago
- Keywords commit removed
@cdog see comments on https://github.com/xwp/wordpress-develop/pull/184
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
#112
@
8 years ago
@westonruter woah, good catch, thanks for fixing!
Thanks for letting me do this. I love it when I'm part of the team. You are great people!
Related post on the forum http://wordpress.org/support/topic/twenty-ten-align-background-to-bottom
I think adding this to customizer is a good idea. I tested background-position: left bottom; and it works in Chromium 20 and Firefox 15. We may need to check if IE8 would support the vertical position.