Skip to content
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

[ACS-8489] Legal Holds keyboard accessibility #4009

Merged

Conversation

g-jaskowski
Copy link
Contributor

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation
  • Other... Please describe:

What is the current behaviour? (You can also link to an open issue here)
https://hyland.atlassian.net/browse/ACS-8489

What is the new behaviour?
Added keyboard accessibility in components reported by SonarCloud.

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

);
component.ngOnInit();
fixture.detectChanges();
badges = fixture.debugElement.queryAll(By.css('.adf-datatable-cell-badge')).map((badge) => badge.nativeElement);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe use harness here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With harness we can only test mat-icon name, tooltip is a property of adf-icon, that can't be tested with harness

let runActionSpy: jasmine.Spy;
beforeEach(() => {
spyOn(appExtensionService, 'getBadges').and.returnValue(
of([{ id: 'test', type: ContentActionType.custom, icon: 'warning', tooltip: 'test tooltip', actions: { click: 'test' } }])
Copy link
Contributor

Choose a reason for hiding this comment

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

I would be into define the object in one place, instead three separate almost the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, fixed, thanks!

@g-jaskowski
Copy link
Contributor Author

SonarCloud started reporting the adf-icon again, after change from keypress -> keypress.enter...

@@ -34,6 +34,12 @@ import { IconComponent, TranslationService } from '@alfresco/adf-core';
import { FormControl, ReactiveFormsModule } from '@angular/forms';
import { switchMap, takeUntil } from 'rxjs/operators';
import { AppExtensionService } from '@alfresco/aca-shared';
import { MatOptionSelectionChange } from '@angular/material/core';

export interface ActionsDropdownSelectionEvent {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export interface ActionsDropdownSelectionEvent {
export interface BulkActionsDropdownSelectionEvent {

@g-jaskowski g-jaskowski merged commit 0fd3683 into ACS-8036-bulk-update Aug 9, 2024
1 check passed
@g-jaskowski g-jaskowski deleted the ACS-8489-Review-Legal-Holds-accessibility branch August 9, 2024 08:58
Copy link

sonarqubecloud bot commented Aug 9, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

DaryaBalvanovich pushed a commit that referenced this pull request Aug 9, 2024
* ACS-8489 handle keyboard accessibility

* ACS-8489 wording fix

* ACS-8489 change function name

* ACS-8489 fix typo

* ACS-8489 review remarks - simplify keyboard event handling, improve unit tests

* ACS-8489 move duplicated code to method

* ACS-8489 change type name to more precise
DaryaBalvanovich pushed a commit that referenced this pull request Aug 12, 2024
* ACS-8489 handle keyboard accessibility

* ACS-8489 wording fix

* ACS-8489 change function name

* ACS-8489 fix typo

* ACS-8489 review remarks - simplify keyboard event handling, improve unit tests

* ACS-8489 move duplicated code to method

* ACS-8489 change type name to more precise
DaryaBalvanovich pushed a commit that referenced this pull request Aug 14, 2024
* ACS-8489 handle keyboard accessibility

* ACS-8489 wording fix

* ACS-8489 change function name

* ACS-8489 fix typo

* ACS-8489 review remarks - simplify keyboard event handling, improve unit tests

* ACS-8489 move duplicated code to method

* ACS-8489 change type name to more precise
DaryaBalvanovich added a commit that referenced this pull request Aug 14, 2024
…ialog with Manage Existing Holds/Apply New Hold tabs (#4019)

* [ACS-8325] [Bulk Legal Hold] Create Bulk Actions Dropdown (#3956)

* ACS-8036 create bulk-actions-dropdown

* ACS-8325 update names and add unit tests

* ACS-8325 added icon and unit tests

* ACS-8325 fix translation

* ACS-8325 add and refactor tests in app extension service

* ACS-8325 resolve conversations: remove loadBulkActions, rename bulk schema, add translations

* ACS-8325 update state

* ACS-8325 temporary remove disabled state as it doesn't work after Angular migration

* ACS-8325 add formControl and tests

* ACS-8325 clean code

* ACS-8325 place create component to one place

* ACS-8325 add condition to use title if no description provided and cover with test

* ACS-8325 add tooltip and update Badge interface

* ACS-8325 rename class names and mockItem object

* [ACS-8326] enable running actions from bulk dropdown and resetting selection (#3971)

* ACS-8036 create bulk-actions-dropdown

* ACS-8325 update names and add unit tests

* ACS-8325 fix translation

* ACS-8325 resolve conversations: remove loadBulkActions, rename bulk schema, add translations

* ACS-8325 temporary remove disabled state as it doesn't work after Angular migration

* ACS-8325 add formControl and tests

* ACS-8325 add condition to use title if no description provided and cover with test

* ACS-8325 add tooltip and update Badge interface

* ACS-8326 enable running actions from bulk dropdown and resetting dropdown selection

* ACS-8326 review remarks - use select control, fix naming

* ACS-8326 fix unit test selectors

* ACS-8326 review remarks - change property name to more universal, adjust unit tests

---------

Co-authored-by: Darya Balvanovich <[email protected]>

* [ACS-8424][Bulk Legal Hold] Add Badge for items (#3985)

* [ACS-8424] display badges in search result

* [ACS-8424] display badges in search result

* ACS-8424 create separate badge component, add/move unit tests, undo highlight change failing tests

* ACS-8424 template cleanup

* ACS-8424 template cleanup cleanup

* ACS-8424 revert single deletion in template

* ACS-8424 rename new component, change property order

---------

Co-authored-by: g-jaskowski <[email protected]>

* ACS-8458 refactor styles (#4018)

* [ACS-8489] Legal Holds keyboard accessibility (#4009)

* ACS-8489 handle keyboard accessibility

* ACS-8489 wording fix

* ACS-8489 change function name

* ACS-8489 fix typo

* ACS-8489 review remarks - simplify keyboard event handling, improve unit tests

* ACS-8489 move duplicated code to method

* ACS-8489 change type name to more precise

* [ACS-8036] fix icon visibility in mat-select

* ACS-8036 fix styles and import

* ACS-8036 remove async from test

* ACS-8036 handle event when dropdown closed

* ACS-8036 fix rebase issues

* ACS-8036 fix rebase issues

* ACS-8036 fix rebase

---------

Co-authored-by: Grzegorz Jaśkowski <[email protected]>
Co-authored-by: tamaragruszka <[email protected]>
Co-authored-by: g-jaskowski <[email protected]>
Co-authored-by: tamaragruszka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants