Skip to content

Commit f3da126

Browse files
committed
[AGILE-361] Drop the visible selection count
Reserving a line above both planning columns shifted every card beneath it, which moved drop geometry enough to break positional drags: the unchanged drag_in_bucket spec failed on reordering to a bucket's first position and on dragging out to the inbox. Design had not settled on needing an on-screen count, so it goes rather than earning a workaround. The shared description stays, renamed with its component: permanently hidden, so it costs no layout, and it is what tells a screen-reader user which cards are in the batch. Counts still announce through the live region. https://community.openproject.org/wp/AGILE-361
1 parent 6d17eb9 commit f3da126

13 files changed

Lines changed: 36 additions & 157 deletions

File tree

frontend/src/global_styles/content/modules/_backlogs.sass

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,7 @@
4444
padding-right: 15px
4545

4646
.op-backlogs-page
47-
// A column so the persistent selection count (a sibling of
48-
// .op-sprint-planning-container, rendered once above both scroll columns)
49-
// stacks above it instead of overlapping it.
50-
display: flex
51-
flex-direction: column
47+
display: block
5248
height: 100%
5349
container-name: backlogsListsContainer
5450
container-type: inline-size
@@ -57,11 +53,7 @@
5753
display: flex
5854
flex-direction: row
5955
gap: var(--stack-gap-normal)
60-
// Shares the page height with the selection count above it rather than
61-
// claiming the full 100% for itself, which would push it past the bottom
62-
// of .op-backlogs-page.
63-
flex: 1 1 auto
64-
min-height: 0
56+
height: 100%
6557
align-items: stretch
6658
overflow: hidden
6759

frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,6 @@ interface MoveAnnouncementContext { label:string|null; listName:string|null; cro
8383
export default class SortableListsController extends Controller<HTMLElement> implements SortableListsRoot {
8484
static outlets = ['sortable-lists--list', 'sortable-lists--item', 'sortable-lists--scrollable'];
8585

86-
static targets = ['selectionCount'];
87-
8886
static values = {
8987
moveUrlTemplate: String,
9088
selectionEnabled: { type: Boolean, default: false },
@@ -101,8 +99,6 @@ export default class SortableListsController extends Controller<HTMLElement> imp
10199
declare readonly selectionEnabledValue:boolean;
102100
declare readonly announcementScopeValue:string;
103101
declare readonly selectionDescriptionIdValue:string;
104-
declare readonly selectionCountTarget:HTMLElement;
105-
declare readonly hasSelectionCountTarget:boolean;
106102

107103
private readonly selection = new BatchSelection();
108104

@@ -883,27 +879,6 @@ export default class SortableListsController extends Controller<HTMLElement> imp
883879

884880
private syncSelectionPresentation():void {
885881
applySelectionPresentation(this.element, this.selection.ids, this.selectionDescriptionIdValue);
886-
this.renderSelectionCount();
887-
}
888-
889-
private renderSelectionCount():void {
890-
if (!this.hasSelectionCountTarget) {
891-
return;
892-
}
893-
894-
const { size } = this.selection;
895-
const empty = size <= 1;
896-
// Its own key, distinct from the `selected` announcement: the on-screen
897-
// count is read alongside the rest of the page's static chrome, not
898-
// spoken once at gesture time, so it drops the announcement's trailing
899-
// period rather than reusing that sentence verbatim.
900-
this.selectionCountTarget.textContent = empty ? '' : this.selectionMessage('count_label');
901-
// A class, not the `hidden` attribute: `hidden` pulls the element out
902-
// of flow, so the planning columns beneath it jump every time the
903-
// selection crosses the one/two-card boundary. The stylesheet keeps
904-
// this element's height reserved at all times and only changes its
905-
// visibility on this class, so toggling it causes no reflow.
906-
this.selectionCountTarget.classList.toggle('op-backlogs-selection-count--empty', empty);
907882
}
908883

909884
private announceSelection(key:'selected'|'cleared'|'not_selectable'|'range_unavailable'|'range_blocked'|'range_restarted'):void {
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1 @@
1-
@import "./backlogs/selection_count_component.sass"
21
@import "./backlogs/work_package_card_list_component.sass"

modules/backlogs/app/components/backlogs/selection_count_component.sass

Lines changed: 0 additions & 17 deletions
This file was deleted.

modules/backlogs/app/components/backlogs/selection_count_component.html.erb renamed to modules/backlogs/app/components/backlogs/selection_description_component.html.erb

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,29 +27,9 @@ See COPYRIGHT and LICENSE files for more details.
2727
2828
++%>
2929

30-
<%=
31-
render(
32-
Primer::Box.new(
33-
tag: :p,
34-
mb: 2,
35-
color: :muted,
36-
font_size: :small,
37-
data: { sortable_lists_target: "selectionCount" },
38-
# Off, not merely the browser default: the root already announces
39-
# selection changes through the shared Primer live region, so this
40-
# region must stay silent or every change would announce twice.
41-
aria: { live: :off },
42-
# Stays in the layout at all times (no `hidden`/`display: none`): the
43-
# stylesheet reserves this element's line height unconditionally, and
44-
# `--empty` only ever toggles `visibility`, so a count appearing or
45-
# disappearing never pushes the planning columns below it up or down.
46-
# The controller flips the same class on selection change; this is
47-
# only the empty-until-JS-runs starting state.
48-
classes: "op-backlogs-selection-count op-backlogs-selection-count--empty"
49-
)
50-
)
51-
%>
52-
30+
<%# Permanently hidden: `aria-describedby` reaches it regardless, and keeping
31+
it out of the visual flow is what leaves the planning columns' geometry
32+
untouched. %>
5333
<%= render(Primer::Box.new(tag: :span, id: DESCRIPTION_ID, hidden: true)) do %>
5434
<%= I18n.t("js.backlogs.selection.card_state") %>
5535
<% end %>

modules/backlogs/app/components/backlogs/selection_count_component.rb renamed to modules/backlogs/app/components/backlogs/selection_description_component.rb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@
2929
#++
3030

3131
module Backlogs
32-
# Persistent visible count of the current batch selection, plus the single
33-
# description that every selected card points at.
32+
# The single description that every selected card points at through
33+
# `aria-describedby`, so batch membership reaches assistive technology per
34+
# card without duplicating the string onto every one of them.
3435
#
35-
# Neither carries server-rendered state: the count is client state and would
36-
# be stale the moment the next card is selected, and the description is one
37-
# shared element so membership reaches assistive technology per card without
38-
# duplicating a string onto every card.
39-
class SelectionCountComponent < ApplicationComponent
36+
# It carries no server-rendered state: selection is client state and would be
37+
# stale the moment the next card is selected. The element is permanently
38+
# hidden, which is also why it costs the planning columns no layout space.
39+
class SelectionDescriptionComponent < ApplicationComponent
4040
DESCRIPTION_ID = "op-backlogs-batch-selected-description"
4141
end
4242
end

modules/backlogs/app/views/backlogs/backlog/_backlog_list.html.erb

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,9 @@ See COPYRIGHT and LICENSE files for more details.
2929

3030
<%# Attributes on this frame tag are ignored; only its children are reconciled — set them in show.html.erb. %>
3131
<%= turbo_frame_tag "backlogs_container" do %>
32-
<%# Rendered once, above both scroll columns: the count has to persist and
33-
stay visible regardless of which column's selection changed or how far
34-
either column has scrolled, and a column's own overflow would carry it
35-
out of view if it lived inside one. %>
36-
<%= render(Backlogs::SelectionCountComponent.new) %>
32+
<%# Rendered once for the whole page: every selected card in either column
33+
points its `aria-describedby` at this one element. %>
34+
<%= render(Backlogs::SelectionDescriptionComponent.new) %>
3735

3836
<div class="op-sprint-planning-container">
3937
<div id="owner_backlogs_container" class="op-sprint-planning-lists" data-controller="sortable-lists--scrollable">

modules/backlogs/app/views/backlogs/backlog/show.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ See COPYRIGHT and LICENSE files for more details.
5454
sortable_lists_selection_enabled_value: true,
5555
sortable_lists_announcement_scope_value: "js.backlogs.selection",
5656
sortable_lists_selection_description_id_value:
57-
Backlogs::SelectionCountComponent::DESCRIPTION_ID,
57+
Backlogs::SelectionDescriptionComponent::DESCRIPTION_ID,
5858
sortable_lists_sortable_lists__list_outlet: "#backlogs_container [data-controller~='sortable-lists--list']",
5959
sortable_lists_sortable_lists__item_outlet: "#backlogs_container [data-controller~='sortable-lists--item']",
6060
sortable_lists_sortable_lists__scrollable_outlet: "#backlogs_container [data-controller~='sortable-lists--scrollable']"

modules/backlogs/config/locales/js-en.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ en:
3333
selection:
3434
card_state: "Selected"
3535
cleared: "Selection cleared."
36-
count_label:
37-
one: "1 work package selected"
38-
other: "%{count} work packages selected"
3936
not_selectable: "Selection unchanged. This work package cannot be selected because it cannot be moved."
4037
range_blocked: "Selection unchanged. That range contains a work package that cannot be moved."
4138
range_restarted:

modules/backlogs/spec/components/backlogs/selection_count_component_spec.rb renamed to modules/backlogs/spec/components/backlogs/selection_description_component_spec.rb

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -30,41 +30,23 @@
3030

3131
require "rails_helper"
3232

33-
RSpec.describe Backlogs::SelectionCountComponent, type: :component do
33+
RSpec.describe Backlogs::SelectionDescriptionComponent, type: :component do
3434
subject(:rendered_component) { render_inline(described_class.new) && page }
3535

36-
# Capybara::Node::Simple (what `render_inline`'s `page` is backed by) only
37-
# ever treats the `hidden` attribute, inline `display: none`, and a few tag
38-
# names as invisible — it never evaluates CSS, so it cannot see the
39-
# `visibility: hidden` this component now hides itself with. Every
40-
# assertion below therefore uses `visible: :all` rather than `:hidden`:
41-
# under Simple's rules this element reads as "visible" regardless of the
42-
# empty-state class, in both states, so `:hidden` would find nothing.
43-
# The real invisibility is exercised by the feature spec's Selenium
44-
# driver, which does evaluate computed style correctly.
45-
46-
it "renders a count region wired to the sortable-lists root" do
47-
expect(rendered_component).to have_css('[data-sortable-lists-target="selectionCount"]', visible: :all)
48-
end
49-
50-
it "starts empty, because a count of nothing is noise" do
51-
expect(rendered_component)
52-
.to have_css('[data-sortable-lists-target="selectionCount"].op-backlogs-selection-count--empty',
53-
visible: :all)
54-
end
55-
56-
it "never carries the hidden attribute, because that would pull it out of flow and reflow the columns below it" do
57-
expect(rendered_component)
58-
.to have_no_css('[data-sortable-lists-target="selectionCount"][hidden]', visible: :all)
59-
end
60-
61-
it "renders no count text server-side" do
62-
expect(rendered_component.find('[data-sortable-lists-target="selectionCount"]', visible: :all).text).to eq("")
63-
end
36+
# `visible: :all` because the element is permanently `hidden`, which is the
37+
# one form of invisibility Capybara::Node::Simple does honour.
6438

6539
it "renders the description every selected card points at" do
6640
expect(rendered_component)
6741
.to have_css("##{described_class::DESCRIPTION_ID}", text: I18n.t("js.backlogs.selection.card_state"),
6842
visible: :all)
6943
end
44+
45+
# `hidden` is what keeps this out of the planning columns' layout entirely,
46+
# which is the whole reason the visible count that used to sit beside it is
47+
# gone: reserving space above two independently scrolling columns moved the
48+
# cards under the user's cursor mid-drag.
49+
it "keeps the description hidden, since only aria-describedby reaches it" do
50+
expect(rendered_component).to have_css("##{described_class::DESCRIPTION_ID}[hidden]", visible: :all)
51+
end
7052
end

0 commit comments

Comments
 (0)