Skip to content

Path selector accessibility #4621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 14, 2025
Merged

Path selector accessibility #4621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
58e344e
role as navigation and not list
ashton22305 Sep 18, 2025
811a3ac
add more descriptive label to select path button
ashton22305 Sep 23, 2025
c7dd41f
change path selector to use semantic html elements so they can be rea…
ashton22305 Oct 2, 2025
43eef9b
fix tests
ashton22305 Oct 2, 2025
0f05b6f
Merge branch 'master' into path-selector-accessibility
ashton22305 Oct 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/dashboard/app/javascript/path_selector/path_selector_data_table.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class PathSelectorTable {
this.reloadTable(this.initialUrl());

$(`#${this.tableId} tbody`).on('click', 'tr', (event) => { this.clickRow(event) });
$('#favorites').on('click', 'li', (event) => { this.clickRow(event) });
$('#favorites').on('click', 'a', (event) => { this.clickRow(event) });
$(`#${this.breadcrumbId}`).on('click', 'li', (event) => { this.clickBreadcrumb(event) });
$(`#${this.selectButtonId}`).on('click', (event) => { this.selectPath(event) });
}
Expand Down
12 changes: 6 additions & 6 deletions apps/dashboard/app/views/batch_connect/session_contexts/_favorites.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<li role='button'
class='clickable text-wrap list-group-item list-group-item-action'
data-api-url=<%= files_path(path.to_s, fs: path.filesystem) %>>
<%= fa_icon((path.to_s == Dir.home ? 'home' : path.icon), classes: '') %>
<%= path.title || path %>
</li>
<a href="#"
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this should still be ol and li even if the internal is an a.

To me, it makes sense to present it as a list of anchors, much like navbar entries, favorites in the files index and interactive apps in my interactive sessions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was mainly used to follow bootstrap's recommendations. If we use <div> and <a> for lists with interactive elements, list-group handles accessibility for us without a bunch of aria-* attributes.

Here is where they recommend to use these elements in list groups

class='text-wrap list-group-item list-group-item-action link-dark'
data-api-url=<%= files_path(path.to_s, fs: path.filesystem) %>>
<%= fa_icon((path.to_s == Dir.home ? 'home' : path.icon), classes: '') %>
<%= path.title || path %>
</a>
2 changes: 1 addition & 1 deletion apps/dashboard/app/views/batch_connect/session_contexts/_path_selector.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

<%= form.text_field(attrib.id, class: 'form-control', **options) %>

<button type="button" class="btn btn-primary" data-bs-toggle="modal" data-bs-target="#<%= path_selector_id %>">
<button type="button" class="btn btn-primary" data-bs-toggle="modal" data-bs-target="#<%= path_selector_id %>" aria-label="<%= I18n.t('dashboard.select_path_for', element: attrib.label) %>">
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately this may not be an improvement, I like the idea of relating it to the form item label, but this gets read out as "#{aria-label} button". So if I have a form item called "working directory", it reads as "Select path for form element working directory button" which might be confusing since it sounds like "working directory button" is the element (when working directory label should really go with the input not the button).

A quick test shows that aria-labelledby will make it read "working directory button", which is even worse. Maybe we just rearrange the message so that it reads "for form item working directory select path button"? I don't love that though and am very open to other solutions. Whatever we go with needs to be less confusing than "select path button" which was the previous behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we can phrase it "Select path button for working directory" to avoid that while keeping a phrase that doesn't sound strange?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though that does have the screen reader not reading "button" last

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I also would prefer it didn't read 'button' last, just not sure if its possible given the label-type format I was noticing. Don't want to discourage you though if you have an idea to get that behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because "button" isn't actually in the string but is just the screen reader describing the element, it might actually be more clear to someone using a screenreader regularly, though I'm not certain about this.

<%= I18n.t('dashboard.select_path') %>
</button>

Expand Down
6 changes: 3 additions & 3 deletions apps/dashboard/app/views/files/_path_selector_breadcrumb.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<% if file_counter == 0 %>
<li class="breadcrumb-item">
<span id="<%= files_path(@filesystem, Dir.home) %>" class="fa fa-home clickable"></span>
<a href="#" id="<%= files_path(@filesystem, Dir.home) %>" class="fa fa-home icon-link link-dark" aria-label="<%= I18n.t('dashboard.path_selector_home') %>"></a>
</li>
<li class="breadcrumb-item">
<span id="<%= files_path(@filesystem, full_path.parent) %>" class="fa fa-arrow-up clickable"></span>
<a href="#" id="<%= files_path(@filesystem, full_path.parent) %>" class="fa fa-arrow-up icon-link link-dark" aria-label="<%= I18n.t('dashboard.path_selector_parent_directory') %>"></a>
</li>
<% end %>

Expand All @@ -13,6 +13,6 @@
</li>
<% else %>
<li class="breadcrumb-item">
<span id="<%= files_path(@filesystem, file) %>" class="clickable"><%= path_segment_with_slash(@filesystem, file.basename.to_s, file_counter, file_count) %></span>
<a href="#" id="<%= files_path(@filesystem, file) %>"><%= path_segment_with_slash(@filesystem, file.basename.to_s, file_counter, file_count) %></a>
</li>
<% end %>
14 changes: 8 additions & 6 deletions apps/dashboard/app/views/shared/_path_selector_table.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<div
class="modal fade"
id="<%= path_selector_id %>" tabindex="-1" role="dialog"
aria-labelledby="modal-path-selector" aria-hidden="true"
aria-labelledby="modal-path-selector"
data-show-files="<%= show_files %>"
data-show-hidden="<%= show_hidden %>"
data-initial-directory="<%= initial_directory %>"
Expand All @@ -36,25 +36,27 @@
<div class="col-sm-5">
<div class="panel panel-default">
<div class="panel-heading">Favorites</div>
<ol id="favorites" class="list-group text-nowrap">
<div id="favorites" class="list-group text-nowrap">
<% homedir = FavoritePath.new(Dir.home, title:'Home Directory') %>
<%= render(partial: 'favorites', collection: favorites.unshift(homedir), as: :path) %>
</ol>
</div>
</div>
</div>
<% end %>

<div class="<%= favorites ? 'col-sm-7' : 'col-sm-12' %>">
<ol id="<%= breadcrumb_id %>" class="breadcrumb breadcrumb-no-delimiter rounded">
</ol>
<nav>
<ol id="<%= breadcrumb_id %>" class="breadcrumb breadcrumb-no-delimiter rounded">
</ol>
</nav>

<div class="d-none alert alert-warning" role="alert" id="forbidden-warning">
<%= t('dashboard.batch_connect_sessions_path_selector_forbidden_error') %>
</div>

<div class="d-flex justify-content-center" id="<%= table_id %>_spinner">
<div class="loading-icon spinner-border rem-5" role="status">
<span class="sr-only">Loading...</span>
<span class="sr-only" aria-live="polite">Loading...</span>
</div>
</div>
<table class="table table-hover table-condensed d-table w-100" id="<%= table_id %>">
Expand Down
4 changes: 4 additions & 0 deletions apps/dashboard/config/locales/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ en:
nav_sessions: My Interactive Sessions
nav_user: Logged in as %{username}
not_grouped: Other Apps
path_selector_home: home
path_selector_parent_directory: parent directory
path_selector_default_popup_title: 'Select Your Working Directory'
pinned_apps_caption_html: A featured subset of <a href="%{all_apps_url}">all available apps</a>
pinned_apps_category: Apps
Expand All @@ -240,6 +242,8 @@ en:
save: Save
saved_settings_title: Saved Settings
select_path: Select Path
select_path_for: |
%{element} select path
settings_invalid_request: Invalid settings submitted
settings_updated: Settings updated
shared_apps_caption: Shared by %{owner_title} (%{owner})
Expand Down
2 changes: 1 addition & 1 deletion apps/dashboard/test/system/batch_connect_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1760,7 +1760,7 @@ def check_visibility(hidden_id, expect_hidden)

def get_favorites
# For debugging flaky tests
all('#favorites li', wait: 30)
all('#favorites a', wait: 30)
# puts "FAVORITES: "
# puts favorites.map{|i| i['innerHTML']}.join('')
end
Expand Down