CONSOLE-2396: Add skip to content component #6596

jessiehuff · 2020-09-11T13:34:50Z

Currently keyboard users have to tab through the entire top nav and then the side nav in order to get to the main content. Adding a skip to content allows keyboard users to quickly and easily get to the main content of a page and helps support section 2.4.1 Bypass Blocks of WCAG and the VPAT. It's also only visible if you're using a keyboard so it shouldn't negatively affect anything visually. (When the page loads, press Tab. Focus should move to the skip to content component. Press enter and focus will shift to the main content.)

CONSOLE-2396

openshift-ci-robot · 2020-09-11T13:35:13Z

Hi @jessiehuff. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jcaianirh · 2020-09-11T13:41:00Z

/hold

jessiehuff · 2020-09-11T13:59:17Z

Something I noticed in adding this component is that when the skip to content is used, it almost looks like a page refresh. I'm wondering if this has something to do with the way that routing is handled? I notice that this doesn't happen in any PF page demos with the skip to content.

frontend/public/components/app.jsx

spadgett · 2020-09-11T14:25:38Z

Something I noticed in adding this component is that when the skip to content is used, it almost looks like a page refresh. I'm wondering if this has something to do with the way that routing is handled?

Yes, we use react-router. You see that when you use an a element instead of Link. You probably need to use HashLink in this case. If the PF component doesn't allow it -- it looks like it doesn't -- we might have to roll our own.

jessiehuff · 2020-09-11T18:31:20Z

Nice call out on that!! I created an issue in PF. It would make sense to me for our skip to content component to be able to accept other router components instead of always rendering as an anchor tag. I think we already do something similar in our Nav so I'm hoping it shouldn't be too difficult to add that.

jcaianirh

looks good thanks @jessiehuff

jessiehuff · 2020-10-22T14:10:04Z

I've tested this update in VO, NVDA, and JAWS, and it seems to work in all three screen readers.

spadgett · 2020-10-22T14:21:55Z

Hi, @jessiehuff, do we still have the page refresh problem? Do we need to bump Patternfly?

jessiehuff · 2020-10-22T14:44:52Z

Hi, @jessiehuff, do we still have the page refresh problem? Do we need to bump Patternfly?

Hi @spadgett! I haven't seen the page refresh problem since updating the href to href={`${this.props.location.pathname}#content`}. After looking into it on the patternfly side, someone from the team noticed that it likely wasn't originally working because of the <base href="/" /> tag that OpenShift includes. I don't think we'll need to bump patternfly.

spadgett · 2020-10-23T15:08:03Z

Hi, @jessiehuff, do we still have the page refresh problem? Do we need to bump Patternfly?

Hi @spadgett! I haven't seen the page refresh problem since updating the href to href={`${this.props.location.pathname}#content`}. After looking into it on the patternfly side, someone from the team noticed that it likely wasn't originally working because of the <base href="/" /> tag that OpenShift includes. I don't think we'll need to bump patternfly.

Does href="#content" work?

jessiehuff · 2020-10-23T17:05:28Z

@spadgett Unfortunately that's what I was trying previously which gave me the refresh issue. I think OpenShift sets the base href here which becomes <base href="/" /> so that when I use the relative anchor tag like href="#content" on child routes like /dashboard it becomes /#content instead of /dashboard#content.

TheRealJon

/lgtm

spadgett · 2021-01-08T18:42:32Z

/ok-to-test
/retest

spadgett · 2021-01-08T18:43:02Z

frontend/public/components/app.jsx

@@ -159,6 +159,11 @@ class App_ extends React.PureComponent {
                onPerspectiveSelected={this._onNavSelect}
              />
            }
+            skipToContent={
+              <SkipToContent href={`${this.props.location.pathname}#content`}>


This will drop any query parameters in the URL I believe.

Very good point, I hadn't considered that. I just updated it to add that consideration!

TheRealJon

/lgtm

yapei · 2021-01-13T07:26:07Z

/label qe-approved

ahardin-rh · 2021-01-14T19:54:42Z

/label docs-approved

spadgett · 2021-01-14T20:56:38Z

/approve

openshift-ci-robot · 2021-01-14T20:57:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcaianirh, jessiehuff, spadgett, TheRealJon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

reestr · 2021-01-15T13:58:30Z

/label px-approved

spadgett · 2021-01-15T17:19:05Z

/hold cancel

openshift-bot · 2021-01-16T00:05:23Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-01-16T00:57:22Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot added the component/core Related to console core functionality label Sep 11, 2020

openshift-ci-robot requested review from jeff-phillips-18 and TheRealJon September 11, 2020 13:35

openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 11, 2020

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 11, 2020

spadgett reviewed Sep 11, 2020

View reviewed changes

frontend/public/components/app.jsx Outdated Show resolved Hide resolved

jessiehuff mentioned this pull request Sep 25, 2020

feat(SkipToContent): Update skip to content to support react router patternfly/patternfly-react#4831

Closed

jessiehuff force-pushed the feat/skip-to-content branch 3 times, most recently from 483fb76 to a9ba397 Compare September 28, 2020 18:24

jcaianirh approved these changes Sep 28, 2020

View reviewed changes

spadgett changed the title ~~Add skip to content component~~ CONSOLE-2396: Add skip to content component Oct 13, 2020

spadgett added this to the v4.7 milestone Oct 13, 2020

TheRealJon reviewed Jan 7, 2021

View reviewed changes

openshift-ci-robot assigned TheRealJon Jan 7, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 7, 2021

openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 8, 2021

spadgett reviewed Jan 8, 2021

View reviewed changes

jessiehuff force-pushed the feat/skip-to-content branch from a9ba397 to a98234a Compare January 12, 2021 19:09

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 12, 2021

TheRealJon self-requested a review January 12, 2021 19:10

TheRealJon removed their assignment Jan 12, 2021

Add skip to content component

9a9d49e

jessiehuff force-pushed the feat/skip-to-content branch from a98234a to 9a9d49e Compare January 12, 2021 19:21

TheRealJon assigned reestr, yapei and ahardin-rh Jan 12, 2021

TheRealJon approved these changes Jan 12, 2021

View reviewed changes

openshift-ci-robot assigned TheRealJon Jan 12, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 12, 2021

openshift-ci-robot added the qe-approved Signifies that QE has signed off on this PR label Jan 13, 2021

openshift-ci-robot added the docs-approved Signifies that Docs has signed off on this PR label Jan 14, 2021

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 14, 2021

openshift-ci-robot added the px-approved Signifies that Product Support has signed off on this PR label Jan 15, 2021

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 15, 2021

openshift-merge-robot merged commit 13a53ac into openshift:master Jan 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CONSOLE-2396: Add skip to content component #6596

CONSOLE-2396: Add skip to content component #6596

jessiehuff commented Sep 11, 2020

openshift-ci-robot commented Sep 11, 2020

jcaianirh commented Sep 11, 2020

jessiehuff commented Sep 11, 2020

spadgett commented Sep 11, 2020

jessiehuff commented Sep 11, 2020

jcaianirh left a comment

jessiehuff commented Oct 22, 2020

spadgett commented Oct 22, 2020

jessiehuff commented Oct 22, 2020

spadgett commented Oct 23, 2020

jessiehuff commented Oct 23, 2020

TheRealJon left a comment

spadgett commented Jan 8, 2021

spadgett Jan 8, 2021

jessiehuff Jan 12, 2021

TheRealJon left a comment

yapei commented Jan 13, 2021

ahardin-rh commented Jan 14, 2021

spadgett commented Jan 14, 2021

openshift-ci-robot commented Jan 14, 2021

reestr commented Jan 15, 2021

spadgett commented Jan 15, 2021

openshift-bot commented Jan 16, 2021

openshift-bot commented Jan 16, 2021

CONSOLE-2396: Add skip to content component #6596

CONSOLE-2396: Add skip to content component #6596

Conversation

jessiehuff commented Sep 11, 2020

openshift-ci-robot commented Sep 11, 2020

jcaianirh commented Sep 11, 2020

jessiehuff commented Sep 11, 2020

spadgett commented Sep 11, 2020

jessiehuff commented Sep 11, 2020

jcaianirh left a comment

Choose a reason for hiding this comment

jessiehuff commented Oct 22, 2020

spadgett commented Oct 22, 2020

jessiehuff commented Oct 22, 2020

spadgett commented Oct 23, 2020

jessiehuff commented Oct 23, 2020

TheRealJon left a comment

Choose a reason for hiding this comment

spadgett commented Jan 8, 2021

spadgett Jan 8, 2021

Choose a reason for hiding this comment

jessiehuff Jan 12, 2021

Choose a reason for hiding this comment

TheRealJon left a comment

Choose a reason for hiding this comment

yapei commented Jan 13, 2021

ahardin-rh commented Jan 14, 2021

spadgett commented Jan 14, 2021

openshift-ci-robot commented Jan 14, 2021

reestr commented Jan 15, 2021

spadgett commented Jan 15, 2021

openshift-bot commented Jan 16, 2021

openshift-bot commented Jan 16, 2021