fix(a11y): address a11y issues and enable tests (#733)

Co-authored-by: Claire.Nicholas <[email protected]> Co-authored-by: dave vader <[email protected]> Co-authored-by: Kelly Merrick <[email protected]>
go-vela · Nov 2, 2023 · b155c3a · b155c3a
1 parent 607d327
commit b155c3a
Show file tree

Hide file tree

Showing 12 changed files with 79 additions and 58 deletions.
diff --git a/cypress/integration/a11y.lighttheme.spec.js b/cypress/integration/a11y.lighttheme.spec.js
@@ -7,27 +7,41 @@ const A11Y_OPTS = {
     type: 'tag',
     values: ['section508', 'best-practice', 'wcag21aa', 'wcag2aa'],
   },
+  rules: {
+    'page-has-heading-one': { enabled: false },
+  },
 };
 
+const elmExclude = '[style*="padding-left: calc(1ch + 6px)"]';
+
 context('Accessibility (a11y)', () => {
   context('Logged out', () => {
-    it.skip('overview', () => {
-      cy.clearSession();
+    beforeEach(() => {
+      cy.server();
+      cy.route({
+        method: 'GET',
+        url: '/token-refresh',
+        status: 401,
+        response: { message: 'unauthorized' },
+      });
+    });
+
+    it('overview', () => {
       cy.setTheme('theme-light');
       cy.visit('/account/login');
       cy.injectAxe();
       cy.wait(500);
-      cy.checkA11y(A11Y_OPTS);
+      // excludes accessibility testing for Elm pop-up that only appears in Cypress and not on the actual UI
+      cy.checkA11y({ exclude: [elmExclude] }, A11Y_OPTS);
     });
   });
 
   context('Logged in', () => {
     beforeEach(() => {
-      cy.clearSession();
       cy.setTheme('theme-light');
       cy.server();
       // overview page
-      cy.route('GET', '*api/v1/repos*', 'fixture:favorites.json');
+      cy.route('GET', '*api/v1/user*', 'fixture:favorites.json');
       // source repos page
       cy.route(
         'GET',
@@ -58,41 +72,34 @@ context('Accessibility (a11y)', () => {
         'fixture:build_running.json',
       );
     });
-    after(() => {
-      cy.visit('/');
-      cy.server({ enable: false });
-    });
 
-    it.skip('overview', () => {
+    it('overview', () => {
       cy.checkA11yForPage('/', A11Y_OPTS);
     });
 
-    it.skip('source repos', () => {
+    it('source repos', () => {
       cy.checkA11yForPage('/account/source-repos', A11Y_OPTS);
     });
 
-    it.skip('settings', () => {
+    it('settings', () => {
       cy.checkA11yForPage('/github/octocat/settings', A11Y_OPTS);
     });
 
-    it.skip('repo page', () => {
+    it('repo page', () => {
       cy.checkA11yForPage('/github/octocat', A11Y_OPTS);
     });
 
-    it.skip('hooks page', () => {
-      cy.login('/github/octocat/hooks');
-      cy.injectAxe();
-      cy.wait(500);
-      cy.get('[data-test=hook]').click({ multiple: true });
-      cy.checkA11y(A11Y_OPTS);
+    it('hooks page', () => {
+      cy.checkA11yForPage('/github/octocat/hooks', A11Y_OPTS);
     });
 
-    it.skip('build page', () => {
+    it('build page', () => {
       cy.login('/github/octocat/1');
       cy.injectAxe();
       cy.wait(500);
       cy.clickSteps();
-      cy.checkA11y(A11Y_OPTS);
+      // excludes accessibility testing for Elm pop-up that only appears in Cypress and not on the actual UI
+      cy.checkA11y({ exclude: [elmExclude] }, A11Y_OPTS);
     });
   });
 });
diff --git a/cypress/integration/a11y.spec.js b/cypress/integration/a11y.spec.js
@@ -7,22 +7,36 @@ const A11Y_OPTS = {
     type: 'tag',
     values: ['section508', 'best-practice', 'wcag21aa', 'wcag2aa'],
   },
+  rules: {
+    'page-has-heading-one': { enabled: false },
+  },
 };
 
+const elmExclude = '[style*="padding-left: calc(1ch + 6px)"]';
+
 context('Accessibility (a11y)', () => {
   context('Logged out', () => {
-    it.skip('overview', () => {
-      cy.clearSession();
+    beforeEach(() => {
+      cy.server();
+      cy.route({
+        method: 'GET',
+        url: '/token-refresh',
+        status: 401,
+        response: { message: 'unauthorized' },
+      });
+    });
+
+    it('overview', () => {
       cy.visit('/account/login');
       cy.injectAxe();
       cy.wait(500);
-      cy.checkA11y(A11Y_OPTS);
+      // excludes accessibility testing for Elm pop-up that only appears in Cypress and not on the actual UI
+      cy.checkA11y({ exclude: [elmExclude] }, A11Y_OPTS);
     });
   });
 
   context('Logged in', () => {
     beforeEach(() => {
-      cy.clearSession();
       cy.server();
       // overview page
       cy.route('GET', '*api/v1/user*', 'fixture:favorites.json');
@@ -57,36 +71,33 @@ context('Accessibility (a11y)', () => {
       );
     });
 
-    it.skip('overview', () => {
+    it('overview', () => {
       cy.checkA11yForPage('/', A11Y_OPTS);
     });
 
-    it.skip('source repos', () => {
+    it('source repos', () => {
       cy.checkA11yForPage('/account/source-repos', A11Y_OPTS);
     });
 
-    it.skip('settings', () => {
+    it('settings', () => {
       cy.checkA11yForPage('/github/octocat/settings', A11Y_OPTS);
     });
 
-    it.skip('repo page', () => {
+    it('repo page', () => {
       cy.checkA11yForPage('/github/octocat', A11Y_OPTS);
     });
 
-    it.skip('hooks page', () => {
-      cy.login('/github/octocat/hooks');
-      cy.injectAxe();
-      cy.wait(500);
-      cy.get('[data-test=hook]').click({ multiple: true });
-      cy.checkA11y(A11Y_OPTS);
+    it('hooks page', () => {
+      cy.checkA11yForPage('/github/octocat/hooks', A11Y_OPTS);
     });
 
-    it.skip('build page', () => {
+    it('build page', () => {
       cy.login('/github/octocat/1');
       cy.injectAxe();
       cy.wait(500);
       cy.clickSteps();
-      cy.checkA11y(A11Y_OPTS);
+      // excludes accessibility testing for Elm pop-up that only appears in Cypress and not on the actual UI
+      cy.checkA11y({ exclude: [elmExclude] }, A11Y_OPTS);
     });
   });
 });
diff --git a/cypress/support/commands.js b/cypress/support/commands.js
@@ -613,7 +613,8 @@ Cypress.Commands.add('checkA11yForPage', (path = '/', opts = {}) => {
   cy.login(path);
   cy.injectAxe();
   cy.wait(500);
-  cy.checkA11y(opts);
+  // excludes accessibility testing for Elm pop-up that only appears in Cypress and not on the actual UI
+  cy.checkA11y({ exclude: ['[style*="padding-left: calc(1ch + 6px)"]'] }, opts);
 });
 
 Cypress.Commands.add('setTheme', theme => {

diff --git a/src/elm/Help/View.elm b/src/elm/Help/View.elm
@@ -37,7 +37,6 @@ help args =
             (class "details"
                 :: class "help"
                 :: class "-no-pad"
-                :: attribute "role" "button"
                 :: Util.open args.show
             )
             [ summary

diff --git a/src/elm/Main.elm b/src/elm/Main.elm
@@ -2992,7 +2992,7 @@ viewHeader session { feedbackLink, docsLink, theme, help, showId } =
 
         identityAttributeList : List (Html.Attribute Msg)
         identityAttributeList =
-            attribute "role" "navigation" :: Util.open showId
+            Util.open showId
     in
     header []
         [ div [ class "identity", id "identity", Util.testAttribute "identity" ]
@@ -3016,7 +3016,7 @@ viewHeader session { feedbackLink, docsLink, theme, help, showId } =
                     details (identityBaseClassList :: identityAttributeList)
                         [ summary [ class "summary", Util.onClickPreventDefault (ShowHideIdentity Nothing), Util.testAttribute "identity-summary" ] [ text "Vela" ] ]
             ]
-        , nav [ class "help-links", attribute "role" "navigation" ]
+        , nav [ class "help-links" ]
             [ ul []
                 [ li [] [ viewThemeToggle theme ]
                 , li [] [ a [ href feedbackLink, attribute "aria-label" "go to feedback" ] [ text "feedback" ] ]

diff --git a/src/elm/Pages/Build/View.elm b/src/elm/Pages/Build/View.elm
@@ -158,7 +158,7 @@ viewPreview msgs openMenu showMenu now zone org repo showTimestamp build =
 
         buildMenuAttributeList : List (Html.Attribute msg)
         buildMenuAttributeList =
-            [ attribute "role" "navigation", id "build-actions" ] ++ Util.open (List.member build.id openMenu)
+            Util.open (List.member build.id openMenu) ++ [ id "build-actions" ]
 
         restartBuild : Html msgs
         restartBuild =
@@ -196,7 +196,7 @@ viewPreview msgs openMenu showMenu now zone org repo showTimestamp build =
                 details (buildMenuBaseClassList :: buildMenuAttributeList)
                     [ summary [ class "summary", Util.onClickPreventDefault (msgs.toggle (Just build.id) Nothing), Util.testAttribute "build-menu" ]
                         [ text "Actions"
-                        , FeatherIcons.chevronDown |> FeatherIcons.withSize 20 |> FeatherIcons.withClass "details-icon-expand" |> FeatherIcons.toHtml []
+                        , FeatherIcons.chevronDown |> FeatherIcons.withSize 20 |> FeatherIcons.withClass "details-icon-expand" |> FeatherIcons.toHtml [ attribute "aria-label" "show build actions" ]
                         ]
                     , ul [ class "build-menu", attribute "aria-hidden" "true", attribute "role" "menu" ]
                         [ restartBuild
@@ -445,7 +445,7 @@ viewStepDetails model msgs rm step =
                     [ div [ class "-name" ] [ text step.name ]
                     , div [ class "-duration" ] [ text <| Util.formatRunTime model.time step.started step.finished ]
                     ]
-                , FeatherIcons.chevronDown |> FeatherIcons.withSize 20 |> FeatherIcons.withClass "details-icon-expand" |> FeatherIcons.toHtml []
+                , FeatherIcons.chevronDown |> FeatherIcons.withSize 20 |> FeatherIcons.withClass "details-icon-expand" |> FeatherIcons.toHtml [ attribute "aria-label" "show build actions" ]
                 ]
             , div [ class "logs-container" ] [ viewStepLogs msgs.logsMsgs model.shift rm step ]
             ]
@@ -1060,7 +1060,7 @@ followButton followStep resourceType number following =
         , onClick <| followStep toFollow
         , attribute "aria-label" <| tooltip ++ " for " ++ resourceType ++ " " ++ number
         ]
-        [ icon |> FeatherIcons.toHtml [ attribute "role" "img" ] ]
+        [ icon |> FeatherIcons.toHtml [ attribute "role" "img", attribute "aria-label" "show build actions" ] ]
 
 
 {-| viewResourceError : checks for build error and renders message

diff --git a/src/elm/Pages/Hooks.elm b/src/elm/Pages/Hooks.elm
@@ -120,7 +120,7 @@ hooksToRows now hooks org repo redeliverHook =
 -}
 tableHeaders : Table.Columns
 tableHeaders =
-    [ ( Just "-icon", "" )
+    [ ( Just "-icon", "Status" )
     , ( Nothing, "source" )
     , ( Nothing, "created" )
     , ( Nothing, "host" )
@@ -136,44 +136,37 @@ renderHook now org repo redeliverHook hook =
     tr [ Util.testAttribute <| "hooks-row", hookStatusToRowClass hook.status ]
         [ td
             [ attribute "data-label" "status"
-            , scope "row"
             , class "break-word"
             , class "-icon"
             ]
             [ hookStatusToIcon hook.status ]
         , td
             [ attribute "data-label" "source-id"
-            , scope "row"
             , class "no-wrap"
             ]
             [ small [] [ code [ class "source-id", class "break-word" ] [ text hook.source_id ] ] ]
         , td
             [ attribute "data-label" "created"
-            , scope "row"
             , class "break-word"
             ]
             [ text <| (Util.relativeTimeNoSeconds now <| Time.millisToPosix <| Util.secondsToMillis hook.created) ]
         , td
             [ attribute "data-label" "host"
-            , scope "row"
             , class "break-word"
             ]
             [ text hook.host ]
         , td
             [ attribute "data-label" "event"
-            , scope "row"
             , class "break-word"
             ]
             [ text hook.event ]
         , td
             [ attribute "data-label" "branch"
-            , scope "row"
             , class "break-word"
             ]
             [ text hook.branch ]
         , td
             [ attribute "data-label" ""
-            , scope "row"
             , class "break-word"
             ]
             [ a

diff --git a/src/elm/Pages/RepoSettings.elm b/src/elm/Pages/RepoSettings.elm
@@ -246,6 +246,7 @@ badge repo velaAPI velaURL copyMsg =
                     [ class "form-control"
                     , class "copy-display"
                     , class "-is-expanded"
+                    , attribute "aria-label" "status badge markdown code"
                     , rows 2
                     , readonly True
                     , wrap "soft"

diff --git a/src/elm/Search.elm b/src/elm/Search.elm
@@ -57,7 +57,7 @@ homeSearchBar filter search =
             , onInput search
             ]
             []
-        , FeatherIcons.filter |> FeatherIcons.toHtml [ attribute "role" "img" ]
+        , FeatherIcons.filter |> FeatherIcons.toHtml [ attribute "aria-label" "filter" ]
         ]
 
 
@@ -74,7 +74,7 @@ repoSearchBarGlobal searchFilters search =
             , id "global-search-input"
             ]
             []
-        , FeatherIcons.filter |> FeatherIcons.toHtml [ attribute "role" "img" ]
+        , FeatherIcons.filter |> FeatherIcons.toHtml [ attribute "aria-label" "filter" ]
         ]
 
 
@@ -93,7 +93,7 @@ repoSearchBarLocal searchFilters org search =
             , onInput <| search org
             ]
             []
-        , FeatherIcons.filter |> FeatherIcons.toHtml [ attribute "role" "img" ]
+        , FeatherIcons.filter |> FeatherIcons.toHtml [ attribute "aria-label" "filter" ]
         ]
 
 

diff --git a/src/scss/_main.scss b/src/scss/_main.scss
@@ -207,7 +207,7 @@ nav {
     }
 
     &:last-child {
-      color: var(--color-bg-dark);
+      color: var(--color-offwhite);
       font-weight: bold;
 
       background-color: var(--color-lavender);

diff --git a/src/scss/_themes.scss b/src/scss/_themes.scss
@@ -48,6 +48,12 @@ body.theme-light {
 }
 
 .theme-light {
+  .navigation a,
+  .table-base a,
+  .logs-header .button.-link {
+    color: var(--color-lavender-dark);
+  }
+
   .status.success {
     color: var(--color-green-dark);
   }
@@ -162,6 +168,6 @@ body.theme-light {
   }
 
   .table-base .error-content {
-    color: var(--color-red);
+    color: var(--color-red-darkest);
   }
 }
diff --git a/src/scss/_variables.scss b/src/scss/_variables.scss
@@ -6,6 +6,7 @@
   --color-cyan-semi-dark: hsl(192, 100%, 38%);
   --color-cyan: hsl(192, 100%, 50%); // good for text on coal
   --color-cyan-light: hsl(192, 100%, 65%);
+  --color-cyan-lightest: hsl(192, 100%, 81%);
   --color-lavender-dark: hsl(286, 29%, 40%);
   --color-lavender: hsl(286, 29%, 51%); // good for text on offwhite
   --color-lavender-light: hsl(286, 29%, 65%); // good for text on coal
@@ -18,8 +19,10 @@
   --color-offwhite: hsl(0, 0%, 98%); // main light bg
   --color-gray: hsl(0, 0%, 70%);
   --color-gray-light: hsl(0, 0%, 85%);
+  --color-gray-lightest: hsl(0, 0%, 94%);
 
   // status indications
+  --color-red-darkest: hsl(353, 90%, 20%);
   --color-red-dark: hsl(353, 77%, 40%);
   --color-red: hsl(353, 77%, 50%); // passes contrast on light bg
   --color-red-light: hsl(353, 77%, 66%); // passes contrast on coal bg