- From: Kevin White <kevin@w3.org>
- Date: Wed, 24 Sep 2014 10:44:21 +0100
- To: wai-eo-editors@w3.org
- Message-Id: <BD640348-6FEA-4444-AFCB-DC7CA2825F47@w3.org>
Hi Eric, Shadi, Great work as ever… just some thoughts and questions. Kevin Minor things: • Couple of spots the phase ‘for more background’ is used. I would probably suggest ‘for more information’ as being more common. • Might consider putting a link to download the complete example. https://w3c.github.io/wai-tutorials/carousels/structure/#brief-content: It may be more useful to have different alt content and image urls in the example to make it clear these are different slides. Wouldn’t need much more than the addition of a number, e.g. “Alternative test 1”. https://w3c.github.io/wai-tutorials/carousels/structure/#complex-content: "This approach was selected as the individual carousel items are not independent pieces of content and thus not suitable for the <article> element” I don’t know that it is clear what is meant by ‘independent pieces of content’ and why this makes <article> inappropriate. https://w3c.github.io/wai-tutorials/carousels/structure/#carousel-styling: "If scripting is not enabled for various reasons, including bad network connection or user choice, the functionality would not work anyway.” I am not sure this makes sense or is useful. What is the aim of this sentence? "In many cases carousels are styled with background images and overlaid text, so that color contrast ratio considerations are relevant.” Might be a style thing but something like: “In many cases carousels are styled with background images and overlaid text, so ensure that you check color contrast ratios.” https://w3c.github.io/wai-tutorials/carousels/structure/#styled-carousel: It might be more useful to use images that don’t require horizontal scrolling. I know this will change depending on viewport size (the column being 66%). Unhelpfully HTML5 img cannot have a proportional width. Perhaps consider a smaller viewport as the default, these seem to be set for a viewport width of 1302px. This, admittedly, isn’t that large with modern displays but might cause some difficulties. Maybe you could use <picture> :) I think at this point it is worth noting that even without any scripting the content makes sense and is useful. https://w3c.github.io/wai-tutorials/carousels/functionality/: "This functionality is added using scripting, based structure of the elements involved.” I am not sure what this sentence is trying to say. Overall I might be tempted to suggest splitting out the functionality into separate pages for each of the functional sections rather than have it all on one page. https://w3c.github.io/wai-tutorials/carousels/functionality/#scripted-styling: "In the example below, JavaScript is used to add the class name .active to the carousel container. The styling for this class positions all carousel items all on top of each other and hides them.” Couple of things with this. • If the item is hidden, why is it marked ‘active’ as it is not active? • Why is the class ‘active’ needed at all as the same styling could be applied to the class ‘slide’? If this is needed later then it may be useful to allude as to why. • The second sentence is a bit of a trial to read. Perhaps something like “The styling hides all the slides in a stack with only the ‘current’ slide being visible on top”. https://w3c.github.io/wai-tutorials/carousels/functionality/#switching-carousel-items: “Switching carousel items” might be better as “Changing the current item” "Toggle Buttons” might be better as “Previous and next buttons”. That seems clearer and more common. "These particular buttons are visually displayed as arrows that overlay the carousel items.” Image doesn’t appear to be loading. "This is especially important for possibly noisy background images.” might be better as “This is especially important for noisy background images.”. "They also increase in size when they are focused by keyboard to better highlight to keyboard users where the current focus is.” I wonder if as well as increasing size on focus and hover it might be useful to change the background slightly. This makes it much more apparent what has focus. There is no code to show what prevSlide or nextSlide do. https://w3c.github.io/wai-tutorials/carousels/functionality/#indicating-carousel-items: "Indicating carousel items” might be better as “Indicating the number of items” https://w3c.github.io/wai-tutorials/carousels/functionality/#focusing-carousel-items: "The focus should not be set to the carousel item if the toggle buttons are used, as the user may want to skip over several carousel items quickly and would use the position otherwise.” Not sure what this is referring to or means. I had to look for what ’toggle buttons’ was because it wasn’t common terminology (in my head at least). "setFocus = typeof setFocusHere !== 'undefined' ? setFocusHere : false;” Not sure if default false is better than default true… I guess when using pref/next, default false would be required but the carousel selector buttons would want true. Meh. Six and half a dozen I guess. Minor but "slides[new_current].setAttribute('tabindex', '-1’);”, would you set tabindex as an attribute on the slide container element rather than do it in JavaScript? https://w3c.github.io/wai-tutorials/carousels/animations/: Should carousels be set to animate on page load or would this be a bad thing? I am assuming it deliberate that there is little mention of how the animation is achieved? https://w3c.github.io/wai-tutorials/carousels/animations/#pause-when-focused: "if (!hasClass(event.target, 'slide')) {“ Sorry you may have to explain to me why you wouldn’t suspend animation if the slide receives focus? https://w3c.github.io/wai-tutorials/carousels/animations/#finalized-carousel: Not sure what the final example JavaScript is there for.
Received on Wednesday, 24 September 2014 09:46:29 UTC