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

D8CORE-2536: adding aria-label #716

Merged
merged 13 commits into from
Sep 17, 2020
Merged

Conversation

cjwest
Copy link
Member

@cjwest cjwest commented Aug 18, 2020

READY for review.

Summary

  • Adds <span class="su-global-footer__link--external">(link is external)</span> to the external links in the global footer

Needed By (Date)

Soon

Urgency

Not urgent

Steps to Test

  1. Check out this branch in Stanford Basic
  2. run npm install in Stanford Basic
  3. Run npm run publish to populate /decanter in the Stanford Basic templates directory
  4. Inspect global footer to verify <span class="su-global-footer__link--external">(link is external)</span> accompanies every external link in the global footer.

Affected Projects or Products

  • Does this PR impact any particular projects, products, or modules?
    D8Core, Decanter, anything using Decanter

Associated Issues and/or People

See Also

Copy link
Member

@sherakama sherakama left a comment

Choose a reason for hiding this comment

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

Feedback provided.

@yvonnetangsu
Copy link
Member

yvonnetangsu commented Aug 20, 2020

@cjwest I'm curious - would it get repetitive if all the links in the global footer have "external link" announced with them? I'm wonder if it is a possibility to just add it once in a SR only heading that all the navigation elements in the global footer are external?

@cjwest
Copy link
Member Author

cjwest commented Aug 20, 2020

@yvonnetangsu,
That's a good point regarding redundancy. Would you happen to know of any examples where it's announced only once?

@yvonnetangsu
Copy link
Member

yvonnetangsu commented Aug 20, 2020

@yvonnetangsu,
That's a good point regarding redundancy. Would you happen to know of any examples where it's announced only once?

Hey @cjwest :) admittedly I haven't really looked into this. I'm thinking maybe a SR only Heading that says something along the line of "This is a footer with common navigation menus for most Stanford website and consists of mostly external links". Maybe something we could talk to Level Access about in an office hour.

That is a super long heading of course.... so maybe something shorter like "Stanford global footer with common important external links". Hmm.

@cjwest
Copy link
Member Author

cjwest commented Sep 3, 2020

I spoke with Clare O'Keefe at SOAP office hours.
Because people often navigate a site just by the links, putting it on a containing element element won't get attention by the screen reader. It would be best to add the text to every link.

@sherakama
Copy link
Member

I spoke with Clare O'Keefe at SOAP office hours.
Because people often navigate a site just by the links, putting it on a containing element element won't get attention by the screen reader. It would be best to add the text to every link.

Good to know. Thanks for digging into this.

@sherakama sherakama temporarily deployed to Tugboat September 15, 2020 23:56 Destroyed
@sherakama sherakama temporarily deployed to Tugboat September 16, 2020 00:20 Destroyed
@cjwest cjwest assigned yvonnetangsu and unassigned sherakama Sep 16, 2020
@cjwest
Copy link
Member Author

cjwest commented Sep 16, 2020

Hi @yvonnetangsu,
Let me know if this makes sense. I'm assigning this to you to merge because I'm thinking you're more familiar with the global footer. Please let me know if you think differently.

@yvonnetangsu
Copy link
Member

Hi @yvonnetangsu,
Let me know if this makes sense. I'm assigning this to you to merge because I'm thinking you're more familiar with the global footer. Please let me know if you think differently.

HI @cjwest Sure, I'll look at this tomorrow! Thanks.

Copy link
Member

@yvonnetangsu yvonnetangsu left a comment

Choose a reason for hiding this comment

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

@cjwest This looks good to me - I'm testing this in the Decanter developer environment, not on Stanford Basics. It looks like another small PR might be needed to add the (link is external) ally text to the Logo link element as well, since the logo link is included in the Global Footer. I'm going to approve this PR though. Thanks for working on this.

@yvonnetangsu yvonnetangsu merged commit 2aa311e into master Sep 17, 2020
@yvonnetangsu yvonnetangsu deleted the D8CORE-2536-external-link branch September 17, 2020 00:19
@cjwest
Copy link
Member Author

cjwest commented Sep 17, 2020

Oh thank you @yvonnetangsu!
Good catch! I created another ticket for the logo:
https://stanfordits.atlassian.net/browse/D8CORE-2730

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants