Added XDMoD analytics metrics to jobs widget #3789
Conversation
|
Some screenshots: 
|
| }); | ||
|
|
||
| return rows; | ||
| } | ||
|
|
||
| function analyticsTable(analyticsContent) { |
There was a problem hiding this comment.
I think nested tables like this are an accessibility issue. I.e., they're confusing to navigate through.
I'd guess we need an additional column before id column and within it just text.
|
Looked this over and it's mostly good. Works well. I just have an issue with the HTML structure it creates - I don't think we should nest tables here. |
|
Changes to the look and feel to removed the nested tables: 
|
abujeda
force-pushed
the
998_xdmod_jobs_widget_enhancements
branch
from
5b62ec5 to
bf7bb3a
Compare
|
Sorry I lost the thread on this. Is it ready? I see you continue to make commits. |
|
Yes, it is now ready. |
abujeda
force-pushed
the
998_xdmod_jobs_widget_enhancements
branch
from
5254d55 to
4d536b4
Compare
|
Fixed conflicts - ready to go |
abujeda
force-pushed
the
998_xdmod_jobs_widget_enhancements
branch
from
4d536b4 to
30ff532
Compare
| thead.innerHTML = '<tr> \ | ||
| <th></th> \ |
There was a problem hiding this comment.
This should have a heading. You can use the class sr-only, but I think actual text here could be nice too (if it's a bit short/succinct).
|
|
||
| rows.push(tr); | ||
|
|
||
| // Add job analytics placeholder | ||
| const analyticsRow = document.createElement('tr'); |
There was a problem hiding this comment.
I think there's something odd about creating a new table row here. Screen readers will get tripped up when row N (a row they've looked at) is now suddenly different because it was the initial row, but after this expansion it's now this new data.
Shouldn't it put all this stuff in the original td and just use some styling to achieve the same look & feel?
There was a problem hiding this comment.
I can improve the accessibility of the hidden rows with aria-hidden and aria-expanded and aria-controls. Wouldn't that address the screen readers issue?
I am not sure how feasible is to add content to the original td that contains the +/- icon and make it break out of the existing table structure, push the other rows down, and appear in the same way. Or even to have a hidden td and do the same. Will need to investigate.
It would be more feasible to appear as a small pop up / tooltip, but I think that would be less user friendly.
There was a problem hiding this comment.
I can improve the accessibility of the hidden rows with aria-hidden and aria-expanded and aria-controls. Wouldn't that address the screen readers issue?
No it's the fact that it's a tr that's being created. Imagine this - you saw row 2 which the screen reader read out as row 2. Then you expand this thing in row 1 and now all the sudden row 2 is this new information, not what you'd previously read through.
I am not sure how feasible is to add content to the original td that contains the +/- icon and make it break out of the existing table structure, push the other rows down, and appear in the same way.
Yea I'm not sure it'd be easy with the way it is now, but I'm sure there's a flex-flow or flex-direction trick we can employ here. Or maybe it'd be easier if expanding the td shifts everything to the right down so the new information shows on top of the row instead of on the bottom?
There was a problem hiding this comment.
I have removed the extra tr for the analytics data. I am using now a td to render and display the contents.
| <th class="id">ID</th> \ | ||
| <th class="name">Name</th> \ | ||
| <th class="date">Date</th> \ | ||
| <th class="sr-only">Analytics</th> \ |
There was a problem hiding this comment.
Just to be clear - the button is in the 1st column and that expands that data in the 5th column?
I think there's still something off about this. The 5th column reads as blank if you've ever expanded it before and collapsed it. And if you've never collapsed it, it skips the column entirely, when it should read something like analytics column <something about needing to expand it>.
That said - I can merge this now and pick it up later just because this has dragged on for a while.
There was a problem hiding this comment.
Just to be clear - the button is in the 1st column and that expands that data in the 5th column?
Yes , that is the solution I came up with to display the analytics without creating a new row.
Added analytics metrics from XDMoD to the jobs widget.
The analytics are added on demand, with an AJAX call to each job. This call is triggered when the user clicks on one of the jobs.
Fixes #998