Skip to content

start to refactor extened active jobs data#5181

Open
johrstrom wants to merge 3 commits intomasterfrom
active-jobs-accessibility
Open

start to refactor extened active jobs data#5181
johrstrom wants to merge 3 commits intomasterfrom
active-jobs-accessibility

Conversation

@johrstrom
Copy link
Contributor

Starts the work on #734.

This change makes the details for a given job appear to the left of the table - outside of the table. I believe this is the only way to make this functionality accessible - by pushing it outside of any row.

This does not complete this work, it only starts it. I believe we still have to tackle these things in subsequent PRs:

  • the details should likely refactored and put elsewhere
    • should probably go above the table and be sticky so visual users can scroll and still see the info
    • should not be another table this is probably confusing. Probably dl items that flow left to right instead of top
      to bottom
    • probably needs a landmark to easily navigate to for screen-readers
  • javascript needs better error handling
  • javascript probably needs to ariaNotify

@Bubballoo3
Copy link
Contributor

Bubballoo3 commented Mar 20, 2026

I can't seem to navigate to the extended details column via keyboard, I know you mention in the description that this is incomplete, but I wonder if that is something that could be done in this first stage (and kept in future iterations)?

javascript probably needs to ariaNotify

ariaNotify would read the contents to the user, but we probably want it in an easily navigable form if we are changing it. My thought is to start with sending focus there like we did with #5160, which gives more control over how much and what rate the content is read.

I also notice that when errors are encountered with the details page the error message pops up in the same pane as the details would have. I feel like it makes more sense if these appear at the top of the page like all other alerts.

In general, I think each of these things are desirable independent of stylistic considerations of where/how the details appear, but how the different steps are divided is up to you. It would be nice though if we establish verifiable goals for each step.

@johrstrom
Copy link
Contributor Author

OK I can fix the button press here and now.

For the other related issues - I can create subtickets off of #734.

I also notice that when errors are encountered with the details page the error message pops up in the same pane as the details would have. I feel like it makes more sense if these appear at the top of the page like all other alerts.

I am just now becoming aware of this behavior. Never encountered it in the wild until I had to fix that test case. In any case, this is how the current behavior is now.

I'm going to defer that change because the response is 200 OK with the alert-warning embedded in the valid response (i.e., not an error response).

image


$.getJSON(jobDataUrl, function (data) {
// Open this row
row.child(data.html_extended_layout).show();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By abandoning the extended layout convention from before, we lose the node view with grafana/ganglia graphs. IMO it makes more sense to modify the existing setup of rendering layouts for each job rather than abandon it completely, just to retain the same flow of data that we had before. Its fine if we do want to change it though, we should just replace it with a different flow that positions us well for the final state we have in mind.

Additionally, we should likely have a test to ensure graphs are displayed. Making a ticket for that now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By abandoning the extended layout convention from before, we lose the node view with grafana/ganglia graphs.

I know they're missed here, but I'm trying to make the change as small as I can to iterate back to parity.

IMO it makes more sense to modify the existing setup of rendering layouts for each job rather than abandon it completely, just to retain the same flow of data that we had before.

Where can they go though? They can't be in the table itself I don't think. The thrust of the original ticket is having a complex/dynamic table structure is confusing. More confusingly, there's a new table within this newly created row.

I don't think we can add rows that don't follow the same column structure (or have another table within the row lol).

@github-project-automation github-project-automation bot moved this from Awaiting Review to Changes Requested in PR Review Pipeline Mar 20, 2026
@Bubballoo3
Copy link
Contributor

I'm going to defer that change because the response is 200 OK with the alert-warning embedded in the valid response (i.e., not an error response).

It seems to me that not giving an error response there is the larger issue. Strikingly similar to #5179. But I have no issue with deferring that for later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Changes Requested

Development

Successfully merging this pull request may close these issues.

3 participants