Skip to content

Commit

Permalink
feat(adhoc-tools-accessible-names): remove feature flag (#5971)
Browse files Browse the repository at this point in the history
#### Details

<!-- Usually a sentence or two describing what the PR changes -->
This PR removes the feature flag under which development of the accessible names feature took place. Although the main goal of this PR is to remove the feature flag, it also contains very small changes related to the "display-accessible-names" rule; with respect to that, this PR adds another native element attribute (summary, to be specific,) check for elements that get their names from author-provided attributes. The PR also includes the addition of the "section" element to the selectors we query.
##### Motivation

<!-- This can be as simple as "addresses issue #123" -->

##### Context
The "summary" attribute check was added to this PR because some elements like table can get their accessible name from the summary attribute. For more information, please refer to PR #5960 
<!-- Are there any parts that you've intentionally left out-of-scope for a later PR to handle? -->

<!-- Were there any alternative approaches you considered? What tradeoffs did you consider? -->

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [ ] Addresses an existing issue: #0000
- [x] Ran `yarn null:autoadd`
- [x] Ran `yarn fastpass`
- [x] Added/updated relevant unit test(s) (and ran `yarn test`)
- [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`.
- [x] (UI changes only) Added screenshots/GIFs to description above
- [ ] (UI changes only) Verified usability with NVDA/JAWS
![rFF](https://user-images.githubusercontent.com/81589466/187256089-984163a5-e904-4e9a-971f-0521e48d54cf.png)
  • Loading branch information
ZakiyaN0 authored Aug 29, 2022
1 parent a8139d3 commit fdf77b2
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 228 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export const AccessibleNamesAdHocVisualization: VisualizationConfiguration = {
testViewType: 'AdhocStatic',
key: accessiblenamesTestKey,
testMode: TestMode.Adhoc,
featureFlagToEnable: 'showAccessibleNames',
getStoreData: data => data.adhoc[accessiblenamesTestKey],
enableTest: data => (data.adhoc[accessiblenamesTestKey].enabled = true),
disableTest: data => (data.enabled = false),
Expand Down
9 changes: 0 additions & 9 deletions src/common/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export class FeatureFlags {
public static readonly manualInstanceDetails = 'manualInstanceDetails';
public static readonly debugTools = 'debugTools';
public static readonly exportReportOptions = 'exportReportOptions';
public static readonly showAccessibleNames = 'showAccessibleNames';
}

export interface FeatureFlagDetail {
Expand Down Expand Up @@ -98,14 +97,6 @@ export function getAllFeatureFlagDetails(): FeatureFlagDetail[] {
isPreviewFeature: true,
forceDefault: false,
},
{
id: FeatureFlags.showAccessibleNames,
defaultValue: false,
displayableName: 'Show accessible names',
displayableDescription: 'Displays the accessible names of user interface elements',
isPreviewFeature: false,
forceDefault: false,
},
];
}

Expand Down
19 changes: 2 additions & 17 deletions src/popup/components/ad-hoc-tools-panel.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,15 @@
padding-right: 8px;
padding-left: 8px;

%grid {
.ad-hoc-tools-grid {
display: grid;
grid-auto-flow: column;
grid-template-columns: auto auto;
grid-template-rows: auto 1px auto 1px auto 1px auto;
grid-column-gap: 30px;
grid-row-gap: 14px;
}

.ad-hoc-tools-grid {
@extend %grid;

.new-row-needed {
@extend %grid;

grid-template-rows: auto 1px auto 1px auto 1px auto;
}

.no-row-needed {
@extend %grid;

grid-template-rows: auto 1px auto 1px auto;
}
}

.divider {
border-bottom: 1px solid $neutral-8;
}
Expand Down
15 changes: 2 additions & 13 deletions src/popup/components/ad-hoc-tools-panel.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { css, Icon, Link } from '@fluentui/react';
import { FeatureFlags } from 'common/feature-flags';
import { NamedFC } from 'common/react/named-fc';
import { FeatureFlagStoreData } from 'common/types/store-data/feature-flag-store-data';
import { flatMap } from 'lodash';
import * as React from 'react';
import styles from './ad-hoc-tools-panel.scss';
Expand All @@ -12,7 +10,6 @@ import { DiagnosticViewToggleFactory } from './diagnostic-view-toggle-factory';
export interface AdHocToolsPanelProps {
backLinkHandler: () => void;
diagnosticViewToggleFactory: DiagnosticViewToggleFactory;
featureFlagStoreData: FeatureFlagStoreData;
}

const toggleShouldNotHaveDivider = (
Expand All @@ -24,8 +21,6 @@ const toggleShouldNotHaveDivider = (
};

export const AdHocToolsPanel = NamedFC<AdHocToolsPanelProps>('AdHocToolsPanel', props => {
let rowStyle: string = styles.noRowNeeded;

const getTogglesWithDividers = () => {
const toggles = props.diagnosticViewToggleFactory.createTogglesForAdHocToolsPanel();

Expand All @@ -35,11 +30,7 @@ export const AdHocToolsPanel = NamedFC<AdHocToolsPanelProps>('AdHocToolsPanel',
<span key={`divider-${dividerIndex++}`} className={styles.divider}></span>
);

let totalRows = 3;
if (props.featureFlagStoreData[FeatureFlags.showAccessibleNames]) {
totalRows = 4;
rowStyle = styles.newRowNeeded;
}
const totalRows = 4;

const result = flatMap(toggles, (toggle, index) => {
if (toggleShouldNotHaveDivider(index, totalRows, toggles)) {
Expand All @@ -60,9 +51,7 @@ export const AdHocToolsPanel = NamedFC<AdHocToolsPanelProps>('AdHocToolsPanel',

return (
<div className={css('main-section', styles.adHocToolsPanel)}>
<main className={styles.adHocToolsGrid}>
<div className={rowStyle}>{togglesWithDividers}</div>
</main>
<main className={styles.adHocToolsGrid}>{togglesWithDividers}</main>
<div role="navigation" className={styles.adHocToolsPanelFooter}>
<Link
className={styles.link}
Expand Down
9 changes: 6 additions & 3 deletions src/scanner/custom-rules/display-accessible-names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const accessibleNamesConfiguration: RuleConfiguration = {
id: accessibleNamesCheckId,
//this list of roles and elements was derived from the ARIA 1.2 documentation (Section 5.2.8.4: Roles supporting name from author or content): https://www.w3.org/TR/wai-aria-1.2/#namefromauthor
selector:
'[role=alert], [role=alertdialog], [role=application], [role=article], article, [role=banner], [role=blockquote], blockquote, [role=button], button, [role=cell], [role=checkbox], [role=columnheader], [role=combobox], [role=command], [role=complementary], [role=composite], [role=contentinfo], [role=definition], [role=dialog], [role=directory],[role=document], [role=feed], [role=figure], figure, [role=form], form,[role=grid], table, [role=gridcell], td, [role=group], fieldset, [role=heading], h1,h2, h3, h4, h5, h6, [role=img],img,[role=input], [role=landmark], [role=link], a, link, [role=list], [role=listbox], [role=listitem],li, ol, ul, [role=log], [role=main], [role=marquee],[role=math], [role=meter], meter,[role=menu], [role=menubar], [role=menuitem], [role=menuitemcheckbox], [role=menuitemradio], [role=navigation], nav, [role=note], [role=option], option, [role=progressbar], [role=radio], [role=radiogroup], [role=range], [role=region], [role=row], tr, [role=rowgroup], [role=rowheader], th[scope="row"], [role=scrollbar], [role=searchbox], [role=search], [role=select], select, [role=sectionhead], [role=separator], hr, [role=slider], [role=status], [role=spinbutton, [role=switch], [role=tab], [role=table], [role=tablelist], [role=tabpanel], [role=term], td, [role=textbox], textarea, th, [role=time], time, [role=timer], [role=toolbar], [role=tooltip], [role=tree], [role=treegrid], [role=treeitem], input,[role=input],[role=window]',
'[role=alert], [role=alertdialog], [role=application], [role=article], article, [role=banner], [role=blockquote], blockquote, [role=button], button, [role=cell], [role=checkbox], [role=columnheader], [role=combobox], [role=command], [role=complementary], [role=composite], [role=contentinfo], [role=definition], [role=dialog], [role=directory],[role=document], [role=feed], [role=figure], figure, [role=form], form,[role=grid], table, [role=gridcell], td, [role=group], fieldset, [role=heading], h1,h2, h3, h4, h5, h6, [role=img],img,[role=input], [role=landmark], [role=link], a, link, [role=list], [role=listbox], [role=listitem],li, ol, ul, [role=log], [role=main], [role=marquee],[role=math], [role=meter], meter,[role=menu], [role=menubar], [role=menuitem], [role=menuitemcheckbox], [role=menuitemradio], [role=navigation], nav, [role=note], [role=option], option, [role=progressbar], [role=radio], [role=radiogroup], [role=range], [role=region], [role=row], tr, [role=rowgroup], [role=rowheader], th, [role=scrollbar], [role=searchbox], [role=search], [role=select], select, [role=sectionhead],section, [role=separator], hr, [role=slider], [role=status], [role=spinbutton, [role=switch], [role=tab], [role=table], [role=tablelist], [role=tabpanel], [role=term], td, [role=textbox], textarea, th, [role=time], time, [role=timer], [role=toolbar], [role=tooltip], [role=tree], [role=treegrid], [role=treeitem], input,[role=input],[role=window]',
enabled: false,
any: [accessibleNamesCheckId],
matches: hasAccessibleName,
Expand All @@ -28,15 +28,18 @@ function hasAccessibleName(node: HTMLElement): boolean {
// this list of roles and elements were derived from the ARIA 1.2 documentation (Section 5.2.8.6: Roles which cannot be named): https://www.w3.org/TR/wai-aria-1.2/#namefromprohibited
const nameProhibitedSelectors: string =
'caption, figcaption, [role=caption], code, [role=code], del, [role=deletion], em, [role=emphasis],[role=generic], ins, [role=insertion], p, [role=paragraph], [role=presentation], [role=strong], strong, sub, sup, [role=subscript], [role=superscript], [role=none]';
const nativeElements: string = 'blockquote, figure, form, li, meter, time,section, tr'; // overwriting axe-cores accessible name value as their calculation tends to return the contents of these elements
// For certain native elements that do not require attributes, but can only accept names from author-provided attributes as in the "if" check below, axe-core tends to get the accessible names from the elements' contents, which is not entirely right, hence the "if" check to overwrite axe's results
const nativeElements: string = 'blockquote, figure, form, li, meter, table, time,section, tr';
if (axe.utils.matchesSelector(node, nameProhibitedSelectors)) {
return false;
}
// overwriting axe-core's accessible name value as their calculation tends to return the contents of these elements
if (
axe.utils.matchesSelector(node, nativeElements) &&
node.hasAttribute('aria-label') === false &&
node.hasAttribute('aria-labelledby') === false &&
node.hasAttribute('title') === false
node.hasAttribute('title') === false &&
node.hasAttribute('summary') === false
) {
return false;
}
Expand Down
1 change: 0 additions & 1 deletion src/tests/unit/tests/background/feature-flags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ describe('FeatureFlagsTest', () => {
[FeatureFlags.manualInstanceDetails]: false,
[FeatureFlags.debugTools]: false,
[FeatureFlags.exportReportOptions]: false,
[FeatureFlags.showAccessibleNames]: false,
};

const featureFlagValueKeys = keys(featureFlagValues);
Expand Down
Loading

0 comments on commit fdf77b2

Please sign in to comment.