-
Notifications
You must be signed in to change notification settings - Fork 1
feat(ngx-layout): WCAG/ARIA for drag-and-drop #270
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.
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
feat(ngx-layout): WCAG/ARIA for drag-and-drop #270
Conversation
@ekincia @WHeirstrate @DenisValcke Once we all given our okay on the default text, we'll look into adding more languages 👍 |
4f9e026
to
eb5f329
Compare
libs/layout/src/lib/abstracts/drag-and-drop/drag-and-drop.service.ts
Outdated
Show resolved
Hide resolved
[cdkDropListEnterPredicate]="beforeDrop.bind(this)" | ||
(cdkDropListDropped)="drop($event)" | ||
> | ||
@for (item of row; track item; let index = $index) { @if (itemTemplateRecord()[item.key]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The formatting seems to be messed up here. Maybe we should prioritise getting the linter set up properly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a thing we need to tackle the moment we bring in all the other packages?
'[attr.tabIndex]': 'tabIndex()', | ||
}, | ||
}) | ||
export class NgxAccessibleDragAndDropItemDirective extends NgxHasFocusDragAndDropDirective { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm gonna be honest, I don't really like the fact that Accessible
is always visible in the services, directives and types. Yes, we focus on the a11y, but should it be this explicitly mentioned in each name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, the sole purpose of these directives, types and services is accessibility... They serve no other purpose than that; I don't understand why we'd not include it in the name? 😅
It's not like we do this with everything we do, this is just 100% intended for accessible drag and drop, I think the intention should be clear here.
libs/layout/src/lib/components/configurable-layout/configurable-layout.component.ts
Show resolved
Hide resolved
return this.containers.find((container) => container.index === index); | ||
} | ||
|
||
ngAfterViewInit() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Member visibility & return type are missing here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an EsLint rule for that somewhere? Might be good to add it to the list ;P
libs/layout/src/lib/directives/drag-and-drop/drag-and-drop-item.directive.ts
Show resolved
Hide resolved
libs/layout/src/lib/services/live-region/live-region.service.ts
Outdated
Show resolved
Hide resolved
libs/layout/src/lib/services/live-region/live-region.service.ts
Outdated
Show resolved
Hide resolved
eb5f329
to
7556932
Compare
85d1c9a
to
54a7571
Compare
libs/layout/src/lib/const/drag-and-drop/drag-and-drop-message.const.ts
Outdated
Show resolved
Hide resolved
54a7571
to
5373e7e
Compare
No description provided.