Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/whole-crabs-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@hashicorp/design-system-components": patch
---

`AdvancedTable` - Fixed an issue where clicking a cell scrolled the page to center that cell

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[todo] Add start and end comments around entry

3 changes: 2 additions & 1 deletion packages/components/src/components/hds/advanced-table/td.gts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export interface HdsAdvancedTableTdSignature {

export default class HdsAdvancedTableTd extends Component<HdsAdvancedTableTdSignature> {
@tracked private _shouldTrapFocus = false;

private _element!: HTMLDivElement;

// rowspan and colspan have to return 'auto' if not defined because otherwise the style modifier sets grid-area: undefined on the cell, which breaks the grid styles
Expand Down Expand Up @@ -121,7 +122,7 @@ export default class HdsAdvancedTableTd extends Component<HdsAdvancedTableTdSign
)
}}
{{@compositeItem disabled=@isCompositeItemDisabled}}
{{hdsScrollIntoViewOnFocus options=(hash block="center" inline="center")}}
{{hdsScrollIntoViewOnFocus}}
Comment thread
zamoore marked this conversation as resolved.
Outdated
...attributes
>
{{yield}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ export default class HdsAdvancedTableTh extends Component<HdsAdvancedTableThSign
)
}}
{{@compositeItem disabled=@isCompositeItemDisabled}}
{{hdsScrollIntoViewOnFocus options=(hash block="center" inline="center")}}
{{hdsScrollIntoViewOnFocus}}
Comment thread
dchyun marked this conversation as resolved.
Outdated
...attributes
>
<HdsLayoutFlex @justify="space-between" @align="center" @gap="8">
Expand Down
Comment thread
zamoore marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ const hdsScrollIntoViewOnFocus =
const scrollOptions = named.options ?? DEFAULT_OPTIONS;

const onFocus = (): void => {
// only scroll for keyboard-driven focus
if (element.matches(':focus-visible') === false) {
Comment thread
zamoore marked this conversation as resolved.
return;
}

element.scrollIntoView(scrollOptions);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
* SPDX-License-Identifier: MPL-2.0
*/

import sinon from 'sinon';
import { module, test } from 'qunit';
import { render } from '@ember/test-helpers';
import { focus, render } from '@ember/test-helpers';

import { HdsAdvancedTableTd } from '@hashicorp/design-system-components/components';

Expand Down Expand Up @@ -92,4 +93,26 @@ module('Integration | Component | hds/advanced-table/td', function (hooks) {
);
assert.dom('#data-test-advanced-table-td').hasAttribute('lang', 'es');
});

test('it should scroll into view on focus using the minimum scroll distance', async function (assert) {
const stub = sinon.stub(HTMLElement.prototype, 'scrollIntoView');

try {
await render(
<template>
<HdsAdvancedTableTd id="data-test-advanced-table-td" tabindex="0" />
</template>,
);

await focus('#data-test-advanced-table-td');

assert.ok(stub.calledOnce, 'scrollIntoView should be called once');

assert.deepEqual(stub.firstCall.args, [
{ block: 'nearest', inline: 'nearest' },
]);
} finally {
stub.restore();
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: MPL-2.0
*/

import sinon from 'sinon';
import { hash } from '@ember/helper';
import { module, test } from 'qunit';
import { render, focus, click, setupOnerror, find } from '@ember/test-helpers';
Expand Down Expand Up @@ -531,4 +532,28 @@ module('Integration | Component | hds/advanced-table/th', function (hooks) {
);
assert.ok(isClicked);
});

test('it should scroll into view on focus using the minimum scroll distance', async function (assert) {
const stub = sinon.stub(HTMLElement.prototype, 'scrollIntoView');

try {
await render(
<template>
<HdsAdvancedTableTh id="data-test-advanced-table-th" tabindex="0">
Artist
</HdsAdvancedTableTh>
</template>,
);

await focus('#data-test-advanced-table-th');

assert.ok(stub.calledOnce, 'scrollIntoView should be called once');

assert.deepEqual(stub.firstCall.args, [
{ block: 'nearest', inline: 'nearest' },
]);
} finally {
stub.restore();
}
});
});
127 changes: 127 additions & 0 deletions showcase/tests/integration/modifiers/hds-scroll-into-view-test.gts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/**
* Copyright IBM Corp. 2021, 2026
* SPDX-License-Identifier: MPL-2.0
*/

import { module, test } from 'qunit';
import { render, find, focus } from '@ember/test-helpers';
import sinon from 'sinon';
import { hash } from '@ember/helper';
import hdsScrollIntoViewOnFocus from '@hashicorp/design-system-components/modifiers/hds-scroll-into-view-on-focus';

import { setupRenderingTest } from 'showcase/tests/helpers';

module(
'Integration | Modifier | hds-scroll-into-view-on-focus',
function (hooks) {
setupRenderingTest(hooks);

hooks.afterEach(function () {
sinon.restore();
});

test('it calls scrollIntoView with default options when focused', async function (assert) {
assert.expect(2);

const stub = sinon.stub(HTMLElement.prototype, 'scrollIntoView');

await render(
<template>
<button id="target" type="button" {{hdsScrollIntoViewOnFocus}}>
Target
</button>
</template>,
);

await focus('#target');

assert.true(stub.calledOnce, 'scrollIntoView is called once on focus');
assert.deepEqual(stub.firstCall.args[0], {
block: 'nearest',
inline: 'nearest',
});
});

test('it calls scrollIntoView with provided options when focused', async function (assert) {
assert.expect(2);

const stub = sinon.stub(HTMLElement.prototype, 'scrollIntoView');

await render(
<template>
<button
id="target"
type="button"
{{hdsScrollIntoViewOnFocus
options=(hash block="center" inline="end" behavior="smooth")
}}
>
Target
</button>
</template>,
);

await focus('#target');

assert.true(stub.calledOnce, 'scrollIntoView is called once on focus');
assert.deepEqual(stub.firstCall.args[0], {
block: 'center',
inline: 'end',
behavior: 'smooth',
});
});

test('it does not call scrollIntoView when focus is pointer-driven', async function (assert) {
assert.expect(1);

const stub = sinon.stub(HTMLElement.prototype, 'scrollIntoView');

await render(
<template>
<button id="target" type="button" {{hdsScrollIntoViewOnFocus}}>
Target
</button>
</template>,
);

const target = find('#target') as HTMLElement;
const matches = sinon.stub(target, 'matches').callThrough();
matches.withArgs(':focus-visible').returns(false);

await focus('#target');

assert.false(
stub.called,
'scrollIntoView is not called for pointer-driven focus',
);
});

test('it does not call scrollIntoView when a descendant is focused', async function (assert) {
assert.expect(1);

const stub = sinon.stub(HTMLElement.prototype, 'scrollIntoView');

await render(
<template>
<div id="parent" {{hdsScrollIntoViewOnFocus}}>
<button id="child" type="button">
Child
</button>
</div>
</template>,
);

// hold the ":focus-visible" guard open, otherwise it short circuites the handler
const target = find('#parent') as HTMLElement;
const matches = sinon.stub(target, 'matches').callThrough();
matches.withArgs(':focus-visible').returns(false);

await focus('#child');

assert.false(
stub.called,
'scrollIntoView is not called when a descendant is focused',
);
});
},
);
78 changes: 0 additions & 78 deletions showcase/tests/integration/modifiers/hds-scroll-into-view.gts

This file was deleted.