Open StommePoes opened 8 years ago
Hi Mallory,
thanks for the review... I made some changes this morning, if you wouldn't mind looking at it again I would certainly appreciate it... Obviously this is still under development and so some items will be missing... and i'm still straightening out the styles in the calendar after some of the semantic refactoring.
:)
thanks again, Jason
On Fri, Feb 5, 2016 at 10:33 AM, Mallory notifications@github.com wrote:
The HTML could be improved in many ways.
First, we can't use this without keyboard. Sad panda :( Let's fix this. I realise our Guidelines page for Elements says you can use a div for a button, but the reason I fight against that is precisely to prevent what we have here: buttons that don't button when we want to button the buttons. Not your fault. Current code:
First I suggest using real buttons. Positive tab indices (1, 2, 3) should be avoided in general on all pages anywhere. We can solve this with real buttons: they are natively focusable. Also, because we have CSS, I would make the order sensible:
Simply making them buttons would make them buttonable, but the middle one is a button and I can't figure out why. Nothing happens when I click it. Will there? If not, it can be a div and doesn't need to be focusable. Since the highlighted td changes when the buttons are clicked, I thought the easiest way to let a non-visual user understand what they were updating was to, if possible, copy the date number as a variable to the offscreen span. The aria-live=polite means each time the user clicks the button, that changed date number can be announced to AT (access technology) without the user needing to move their virtual focus over to check.
I think the chevrons will later be turned into our o-icons, which already has the aria-hidden and font-awesome stuff, so I'd put that tag inside the button and have an offscreen class for non-visual users (I'll ask elements if we want that as some kind of part if the SDK or something teams should do themselves... I'd prefer if we (PDA) did it for you instead).
The highlighted days and current day don't say anything code-wise. They just have a colour. I think this information should either be placed (again in offscreen tags) inside the table (each date that has a highlight has a bit of text saying it's highlighted/part of the assigment week/whatever) or as just a textual statement elsewhere (before or after the table: "Current highlighted day: 5, range from 5 to and including 12 February" or something). If it's in the table, it'll require a non-visual student to go through the days of the table to find which week is highlighted, but if these weeks can be changed via buttons or server-push notifications or something then that might be a good place for them. You guys know more about what this all needs to do than I do.
For the list of assignments:
AssignmentsOught to be, depending on the content level of this part of the page:
Assignments
Each of the dates which relate to the content underneath them would then be h3's. If there's already an h2 about this Assignments somewhere and Assignments and the table fall under it, these might be shifted to h3-assignments and h4 for the dates.
The whole list of dates+assigments seems like a list to me, as well as the list of assignments per date, however that's a more open question of semantic preference for the developer. I personally would at least wrap the whole set of dates+assigments in 1 list (someone called it a listContainer for a reason) and styles would remove bullets or numbers. Also, currently with History 201, midnight, chapter 2, that sounds kinda weird as well. Also, id's are being repeated. They must be unique per page... I think possibly our CI tests would catch that though.
classes etc removed just to more easily show structure. I particularly think it's important that the stuff is grouped in a way all students know what belongs together: History 201Midnight Chapter 2 Math 301Noon Chapter 6 since there's only divs around these, there's no programmatic/semantic way for one to know which belongs to what, other than by guessing the pattern of course, time, chapter.
Tue Feb, 2
History 201
Chapter 2 MidnightMath 301
Chapter 6 NoonTue Feb, 3
English 221
Chapter 3 NoonTue Feb, 4
Spanish 421
Chapter 3: the Spanish Inquisition NoonPhysics 541
Chapter 3: the Physics of Foo MidnightAlso I figured I'd attach a screenshot from Linux/Chromium. [image: upcoming-assignments screenshot] https://cloud.githubusercontent.com/assets/136210/12853898/e55376f2-cc36-11e5-9307-40f7dbb2642d.png
— Reply to this email directly or view it on GitHub https://github.com/Pearson-Higher-Ed/o-upcomingAssignments/issues/2.
Mallory, did you get a chance to look it over again?
I'm going to look at it today.
Thank You!
On Tue, Feb 16, 2016 at 2:47 PM, Mallory notifications@github.com wrote:
I'm going to look at it today.
— Reply to this email directly or view it on GitHub https://github.com/Pearson-Higher-Ed/o-upcomingAssignments/issues/2#issuecomment-184888600 .
Ok: There's still a button called "today" that doesn't do anything when I click it. What should it do? The other two buttons still have no names. They're just called "push button". I need to hear what they do, and all interactive elements need names (the icons are decorative and don't count as names :)
The table headers are letters but it would be nice to have actual text (even if hidden offscreen) to differentiate between T and T, S and S. Right now their meaning is based on visual context. https://www.w3.org/TR/WCAG10-TECHS/#tech-abbreviate-labels
I still don't know, non-visually, that some days are highlighted.
The h3 for assignments is good. However if everyone inside a li (list item) is a heading then that's a sign probably none of them should be headings. Headings "head" content underneath them. I thought the class name was what the whole assignment was about but if not, if it's just a list of dates with classes + specific chapters of those classes, then I'd have them not have any headings since one isn't naming the whole group.
The word order is also still History 201 Tue Feb, 2 Chapter 2 does that make sense with the assignment? (I can't tell but visually it still looks to me like it's really saying History 201, Chapter 2 Tue Feb, 2 )
Also when I tried to copy that text with the mouse, I kept getting sent to google. If the assignments are supposed to be clickable, they need to work with keyboard, and ideally with an actual focusable, clickable native element (a link would make the most sense if it sends a student to a page with that assignment). If it opens something on another pane of the same page, I would go so far as to say clicking an assignment should also move the keyboard focus to that assignment pane as well (the way a skip link beings a user to another part of a page).
I know I'm not aware of what entirely this thing is supposed to do or what context it will be in, but that's the feedback I have for now.
New screenshot if you want (this is Chromium, looks slightly different in Firefox of course)
Thank you again, Mallory... your help is greatly appreciated. :)
the buttons on either side of "today" move the highlighted 7 day range. The "today" button returns that to it's original position based on what the current day is.
We'll work on getting text for the buttons and the days and positioning that offscreen.
The wording and formatting is coming down from parker in UX. So there's not much i can do about that. also, when you click on an assignment, it's supposed to deep link out to the revel assignment. right now i've got it dummied out to google until we find out the location of the data from LAS assignments. We'll work on making that a link as you suggest.
Thank you again for the help, Mallory!
re "The wording and formatting is coming down from parker in UX. " If the visual grouping of subject and chapter are meant to be together while the date sits on the other side, a more sensible source order can be done without changing how it looks.
However I actually can't tell if Subject-Date-Chapter isn't supposed to be how it's read (I just know most sighted people would read the left side first).
"the buttons on either side of "today" move the highlighted 7 day range. The "today" button returns that to its original position..." Ah, mystery solved.
The HTML could be improved in many ways.
First, we can't use this without keyboard. Sad panda :( Let's fix this. I realise our Guidelines page for Elements says you can use a div for a button, but the reason I fight against that is precisely to prevent what we have here: buttons that don't button when we want to button the buttons. Not your fault. Current code:
First I suggest using real buttons. Positive tab indices (1, 2, 3) should be avoided in general on all pages anywhere. We can solve this with real buttons: they are natively focusable. Also, because we have CSS, I would make the order sensible:
Simply making them buttons would make them buttonable, but the middle one is a button and I can't figure out why. Nothing happens when I click it. Will there? If not, it can be a div and doesn't need to be focusable. Since the highlighted td changes when the buttons are clicked, I thought the easiest way to let a non-visual user understand what they were updating was to, if possible, copy the date number as a variable to the offscreen span. The aria-live=polite means each time the user clicks the button, that changed date number can be announced to AT (access technology) without the user needing to move their virtual focus over to check.
I think the chevrons will later be turned into our o-icons, which already has the aria-hidden and font-awesome stuff, so I'd put that <i> tag inside the button and have an offscreen class for non-visual users (I'll ask elements if we want that as some kind of part if the SDK or something teams should do themselves... I'd prefer if we (PDA) did it for you instead).
The highlighted days and current day don't say anything code-wise. They just have a colour. I think this information should either be placed (again in offscreen tags) inside the table (each date that has a highlight has a bit of text saying it's highlighted/part of the assigment week/whatever) or as just a textual statement elsewhere (before or after the table: "Current highlighted day: 5, range from 5 to and including 12 February" or something). If it's in the table, it'll require a non-visual student to go through the days of the table to find which week is highlighted, but if these weeks can be changed via buttons or server-push notifications or something then that might be a good place for them. You guys know more about what this all needs to do than I do.
For the list of assignments:
<div id="header" class="header">Assignments</div>
Ought to be, depending on the content level of this part of the page:
<h2 id="header" class="header">Assignments</h2>
Each of the dates which relate to the content underneath them would then be h3's. If there's already an h2 about this Assignments somewhere and Assignments and the table fall under it, these might be shifted to h3-assignments and h4 for the dates.
The whole list of dates+assigments seems like a list to me, as well as the list of assignments per date, however that's a more open question of semantic preference for the developer. I personally would at least wrap the whole set of dates+assigments in 1 list (someone called it a listContainer for a reason) and styles would remove bullets or numbers. Also, currently with History 201, midnight, chapter 2, that sounds kinda weird as well. Also, id's are being repeated. They must be unique per page... I think possibly our CI tests would catch that though.
classes etc removed just to more easily show structure. I particularly think it's important that the stuff is grouped in a way all students know what belongs together: History 201Midnight Chapter 2 Math 301Noon Chapter 6 since there's only divs around these, there's no programmatic/semantic way for one to know which belongs to what, other than by guessing the pattern of course, time, chapter.
Also I figured I'd attach a screenshot from Linux/Chromium.