Skip to content

Add failing test case for container width change #599

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

Open
gwak wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add failing test case for container width change #599

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
27a34c7
Add failing test case for conatiner width change
gwak Nov 29, 2024
e8561ec
Merge branch 'adopted-ember-addons:master' into repro-issue-container…
gwak Mar 5, 2025
4a1bcdb
Revert some changes on existing test in tes-app
gwak Mar 5, 2025
a58c828
Merge branch 'adopted-ember-addons:master' into repro-issue-container…
gwak Mar 7, 2026
7f43acd
Update test-app/app/controllers/index.js
gwak Mar 7, 2026
672f9ad
Add regression test for container width layout change
gwak Mar 7, 2026
4db2327
Refactor horizontal demo and table structure in index.hbs
gwak Mar 7, 2026
90549cc
Fix syntax error in updateMutable action decorator
gwak Mar 7, 2026
a56e122
Apply suggestions from code review
gwak Mar 7, 2026
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
22 changes: 22 additions & 0 deletions test-app/app/controllers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export default class ModifierController extends Controller {
{ fruit: 'lemon', day: 'Sunday' },
];

@tracked mutableRecords = ['Zero', 'One', 'Two', 'Three', 'Four'];

@action handleDragChange(reordered) {
this.records = reordered;
}
Expand Down Expand Up @@ -76,4 +78,24 @@ export default class ModifierController extends Controller {
set(this, 'model.itemsGrid2', newOrder);
set(this, 'model.dragged', draggedModel);
}

`@action`
updateMutable(newOrder, draggedModel) {
this.mutableRecords = newOrder;
}
Comment thread
gwak marked this conversation as resolved.

@action
removeItemHorizontal(item) {
this.mutableRecords = this.mutableRecords.filter(
(record) => record !== item,
);
}

@action
addItemToHorizontal() {
this.mutableRecords = [
...this.mutableRecords,
`Item ${this.mutableRecords.length + 1}`,
];
}
}
36 changes: 36 additions & 0 deletions test-app/app/styles/app.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,3 +295,39 @@
.word-break {
word-wrap: break-word;
}

.flex {
display: flex;
}

.flex-1 {
flex: 1;
}

.inline-flex {
display: inline-flex;
}

.align-items-center {
align-items: center;
}

.space-between {
justify-content: space-between;
}

.flex-column {
flex-direction: column;
}

.flex-row {
flex-direction: row;
}

.full-width {
width: 100%;
}

.mr-5 {
margin-right: 5px;
}
82 changes: 63 additions & 19 deletions test-app/app/templates/index.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,51 @@
{{~/each}}
</ol>
</section>


<section class='horizontal-demo'>
<h3 class='flex align-items-center'>Horizontal Mutable
&nbsp; &nbsp;<button type='button' {{on 'click' this.addItemToHorizontal}}>Add Item</button>
</h3>

<div class='flex align-items-center full-width'>
<ol
class='flex-1' data-test-horizontal-demo-group
{{sortable-group
this.mutableRecords
direction='x'
onChange=this.updateMutable
itemVisualClass=this.itemVisualClass
handleVisualClass=this.handleVisualClass
groupName='horizontal-add-1'
}}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
>
{{#each this.mutableRecords as |item|~}}
<li data-test-horizontal-demo-handle tabindex='0' {{sortable-item model=item groupName='horizontal-add-1'}}>
<ItemPresenter @item={{item}} />
<button type='button' {{on 'click' (fn this.removeItemHorizontal item)}}>x</button>

@coderabbitai coderabbitai Bot Mar 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add an explicit accessible name to the remove buttons.

Line 90 and Line 106 expose the action as just x, which is not a meaningful button label for assistive tech. Add an aria-label such as Remove {{item}}.

♿ Proposed fix 
-              <button type='button' {{on 'click' (fn this.removeItemHorizontal item)}}>x</button>
+              <button
+                type='button'
+                aria-label={{concat 'Remove ' item}}
+                {{on 'click' (fn this.removeItemHorizontal item)}}
+              >×</button>
...
-              <button type='button' {{on 'click' (fn this.removeItemHorizontal item)}}>x
+              <button
+                type='button'
+                aria-label={{concat 'Remove ' item}}
+                {{on 'click' (fn this.removeItemHorizontal item)}}
+              >×
               </button>

Also applies to: 106-107

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test-app/app/templates/index.hbs` at line 90, The remove buttons currently
render only "x", which is not accessible; update the button elements that call
the removeItemHorizontal and removeItemVertical actions to include an explicit
aria-label (e.g., aria-label="Remove {{item}}") that uses the item variable to
produce a meaningful label for assistive tech, ensuring both the horizontal and
vertical remove buttons get the same treatment so the click handlers
(removeItemHorizontal / removeItemVertical) remain unchanged.

</li>
{{~/each}}
</ol>
<ol
data-test-horizontal-demo-group
{{sortable-group
direction='x'
onChange=this.updateMutable
itemVisualClass=this.itemVisualClass
handleVisualClass=this.handleVisualClass
groupName='horizontal-add-2'
}}>
{{#each this.mutableRecords as |item|~}}
<li data-test-horizontal-demo-handle tabindex='0' {{sortable-item model=item groupName='horizontal-add-2'}}>
<ItemPresenter @item={{item}} />
<button type='button' {{on 'click' (fn this.removeItemHorizontal item)}}>x
</button>
</li>
{{~/each}}
</ol>
</div>
</section>

<section class="grid-demo">
<div class="row">
<div class="col">
Expand All @@ -77,13 +121,13 @@
<div class="row">
<div class="col">
<div class="row" data-test-grid-demo-group
{{sortable-group
direction='grid'
onChange=this.updateGrid
itemVisualClass=this.itemVisualClass
handleVisualClass=this.handleVisualClass
groupName='grid'
}}>
{{sortable-group
direction='grid'
onChange=this.updateGrid
itemVisualClass=this.itemVisualClass
handleVisualClass=this.handleVisualClass
groupName='grid'
}}>
{{#each @model.itemsGrid as |item|~}}
<div class="col-120" data-test-grid-demo-handle tabindex='0' {{sortable-item model=item groupName='grid'}}>
<div class="card">
Expand Down Expand Up @@ -123,18 +167,18 @@
{{! template-lint-disable table-groups}}
<table>
<thead>
<tr>
<th>&vArr;</th>
<th>Item</th>
</tr>
<tr>
<th>&vArr;</th>
<th>Item</th>
</tr>
</thead>
<tbody {{sortable-group onChange=this.update groupName='table'}}>
{{#each @model.items as |item|}}
<tr data-test-table-demo-item class='handle' {{sortable-item model=item groupName='table'}}>
<td><span data-test-table-demo-handle data-item={{item}} class='handle'>&vArr;</span></td>
<td>{{item}}</td>
</tr>
{{/each}}
{{#each @model.items as |item|}}
<tr data-test-table-demo-item class='handle' {{sortable-item model=item groupName='table'}}>
<td><span data-test-table-demo-handle data-item={{item}} class='handle'>&vArr;</span></td>
<td>{{item}}</td>
</tr>
{{/each}}
</tbody>
</table>
</section>
Expand Down Expand Up @@ -225,4 +269,4 @@
</ol>

</section>
</article>
</article>
38 changes: 38 additions & 0 deletions test-app/tests/integration/modifiers/sortable-group-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,44 @@ module('Integration | Modifier | sortable-group', function (hooks) {
assert.dom(announcerSelector).hasText('Cancelling item repositioning');
});

test('firstItemPosition recomputes after external container layout changes', async function (assert) {
this.items = ['One', 'Two'];

this.update = (items) => {
set(this, 'items', items);
};

await render(hbs`
<div id="test-wrapper" style="display:flex;justify-content:center;width:700px;">
<ol
id="test-list"
style="display:block;white-space:nowrap;list-style:none;padding:0;margin:0;width:320px;"
{{sortable-group direction="x" onChange=this.update groupName="external-changes"}}
>
{{#each this.items as |item|}}
<li style="display:inline-block;width:80px;" {{sortable-item model=item groupName="external-changes"}}>
{{item}}
</li>
{{/each}}
</ol>
</div>
`);

const sortableService = this.owner.lookup('service:ember-sortable-internal-state');
const group = sortableService.fetchGroup('external-changes').groupModifier;

const firstPositionBeforeResize = group.firstItemPosition.x;
find('#test-wrapper').style.width = '500px';

group.items.forEach((item) => item.reset());
Comment thread
gwak marked this conversation as resolved.
Outdated
await settled();

const firstItem = group.items[0];
const expectedPositionAfterResize = firstItem.x - firstItem.spacing;
assert.equal(group.firstItemPosition.x, expectedPositionAfterResize);
assert.notEqual(group.firstItemPosition.x, firstPositionBeforeResize);
});

function contents(selector) {
return find(selector)
.textContent.replace(/⇕/g, '')
Expand Down