Skip to content

Feature/accessibility improvements #2

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.

Sign up for GitHub

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

Open
nlaffey wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Feature/accessibility improvements #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
30bc634
Adding default empty string for privileged IP's
nlaffey Dec 27, 2019
9904b40
Adding sass build scripts.
nlaffey Dec 27, 2019
47a7149
Adding label to search input
nlaffey Dec 27, 2019
009e925
Adding skip to main content link and aria label for home link
nlaffey Dec 27, 2019
3de25cb
Adding landmark element and aria-role for department page
nlaffey Dec 27, 2019
9e8cfd0
Fixing header to be h1 header role
nlaffey Dec 27, 2019
119b3cd
Adding keydown handler so images work when pressing enter
nlaffey Dec 27, 2019
a74bc50
Adding main landmark with id for skip to content link
nlaffey Dec 27, 2019
e52c9d9
Updating nav to use landmark and role
nlaffey Dec 27, 2019
ca03124
Temporarily setting rightsstatements to an empty array
nlaffey Dec 27, 2019
70ae3a5
Creating SkipToMainContentLink component
nlaffey Dec 27, 2019
dd4fab9
Adding alt tag for image
nlaffey Dec 27, 2019
cc12c2e
Undoing minor changes to reduce diff
nlaffey Dec 27, 2019
638572c
Updating readme
nlaffey Dec 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ bundle.js
node_modules
css
node_modules-2018-11-27
.idea

@nlaffey nlaffey Dec 27, 2019

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I use intelliJ, this is to prevent my IDE files from being committed

2 changes: 1 addition & 1 deletion index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!doctype html>
<html>
<html lang="en">

@nlaffey nlaffey Dec 27, 2019

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Adding the lang helps non-human readers to identify what language is to be expected from this page.

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/lang

<head>
<meta name="viewport" content="width=device-width, initial-scale=1">
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/leaflet/1.0.0-beta.1/leaflet.css">
Expand Down
3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
"watch": "watchify index.js -p livereactload -o bundle.js -dv",
"watchServer": "node-supervisor --watch server.js,src server.babel.js",
"serve": "PORT=1314 node server.babel.js",
"build-css": "sass ./sass/main.scss ./css/main.css",

@nlaffey nlaffey Dec 27, 2019

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I didn't see any build process for CSS. I've added these scripts and updated the documentation to call it out. Let me know if I overlooked the build process somewhere however.

"build-css:watch": "sass --watch ./sass/main.scss ./css/main.css",
"build": "NODE_ENV=production browserify index.js | uglifyjs -c -m | sed 's|http://localhost:4680|https://search.artsmia.org|'> bundle.js"
},
"author": "",
Expand Down Expand Up @@ -37,6 +39,7 @@
"watchify": "^3.9.0"
},
"dependencies": {
"sass": "^1.24.0",

@nlaffey nlaffey Dec 27, 2019

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

sass dependency for the css build scripts: https://github.com/nlaffey

"@mapbox/react-click-to-select": "^1.1.1",
"babel-polyfill": "^6.23.0",
"babel-preset-es2015": "^6.22.0",
Expand Down
5 changes: 5 additions & 0 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ To start the server locally, you (1) need passing familiarity with
(3) `npm start` should start the webserver and begin to update the
javascript bundle.

## Building SCSS/CSS

To build the css files run `npm run build-css` or alternatively `npm run build-css:watch` to have it watch for changes
and rebuild.

# Companion code

It pulls information from many different places:
Expand Down
31 changes: 30 additions & 1 deletion sass/main.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,28 @@ a:hover {
font-weight: 500;
}

a.skip-main {
left:-999px;
position: fixed;
top:auto;
width:1px;
height:1px;
overflow:hidden;
z-index:-999;
}
a.skip-main:focus, a.skip-main:active {
background-color: white;
position:fixed;
left: 50%;
top: 5px;
width: 200px;
height: 23px;
margin: 10px -100px;
padding:5px;
text-align:center;
z-index:999;
}

#search {
/*margin: 8px;*/
}
Expand Down Expand Up @@ -80,12 +102,19 @@ header{
.quilt-wrap a:hover {
overflow: visible !important;
// z-index: 100;

img {
overflow: visible !important;
}
}

// The regular focus style with the dotted red outline doesn't look very good on the quilt images. A white solid border
// seems to be more visible and doesn't clash as much with the images.
.quilt-wrap a:focus {
outline: 0;
border: 2px solid white;
}

.logo-container{
background-image: url("../images/MIA_LOGO_MARK.svg");
background-repeat: no-repeat;
Expand Down
16 changes: 9 additions & 7 deletions sass/reset.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
/* reset & clearfix */
/* reset & clearfix

@nlaffey nlaffey Dec 27, 2019

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Per my comments here, I've removed the outline styles from the reset. This is something that I would want to run by a designer to see if its suitable for the site. It's important to keep in mind each browser will implement the focus style differently but this may be for the best. Users will be most familiar with their browsers focus style.


Modifications have been to Eric Meyer's Reset:

1. Outline resets have been removed. This is more consistent with artsmia.org website and encourages better accessibility

*/

.clearfix:after {
visibility: hidden;
Expand All @@ -25,7 +31,6 @@ table, caption, tbody, tfoot, thead, tr, th, td {
margin: 0;
padding: 0;
border: 0;
outline: 0;
font-size: 100%;
vertical-align: baseline;
background: transparent;
Expand All @@ -39,10 +44,7 @@ ol, ul {
blockquote, q {
quotes: none;
}
/* remember to define focus styles! */
:focus {
outline: 0;
}

/* remember to highlight inserts somehow! */
ins {
text-decoration: none;
Expand All @@ -62,4 +64,4 @@ html {
}
*, *:before, *:after {
box-sizing: inherit;
}
}
2 changes: 1 addition & 1 deletion server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ var html = ({ title, meta, link }, data, body) => {

app.use((req, res, next) => {
var actingUrl = req.url.replace(/\/(.*)\/$/, '/$1')
const internalIPs = process.env.PRIVILEGED_IP_LIST
const internalIPs = process.env.PRIVILEGED_IP_LIST || '';

@nlaffey nlaffey Dec 27, 2019 •
edited
Loading

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Rather than force users to provide the environment variable I thought it made sense to default to an empty string. If this is set to null then it throws a null pointer error below when we try to .split

var privilegedClientIP = internalIPs.split(' ').indexOf(req.ip) > -1
window.privilegedClientIP = privilegedClientIP // FIXME how to pass this other than as a global?

Expand Down
10 changes: 7 additions & 3 deletions src/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,19 @@ var {pathSatisfies} = require('ramda')
var LiveSearch = require('./live-search')
var GlobalNavigation = require('./navigation')
var consoleWelcomeMessage = require('./console-welcome-message')
var SkipToMainContentLink = require('./skip-to-main-content-link');


var App = React.createClass({
render() {
var logo = this.makeLogo()
var canonicalURL = "https://collections.artsmia.org"+this.props.path
var path = this.props.path
var canonicalURL = "https://collections.artsmia.org" + path

return (
<div className={this.props.universal && 'universal'}>
{this.state.hideHeader || <header style={{zIndex: this.state.showMenu || this.state.showSearch ? 5 : 1}}>
<SkipToMainContentLink path={path} />
{this.state.hideHeader || <header style={{ zIndex: this.state.showMenu || this.state.showSearch ? 5 : 1 }}>
{logo}
{this.globalToolBar()}
</header>}
Expand Down Expand Up @@ -131,7 +135,7 @@ var App = React.createClass({

return showMenu ?
<a href="https://new.artsmia.org" title="Back to artsmia.org">{logo}</a> :
<a href="/" onClick={this.toggleMenu}>{logo}</a>
<a href="/" onClick={this.toggleMenu} aria-label="Show Menu" title="Show Menu">{logo}</a>
},

noIndex() {
Expand Down
17 changes: 11 additions & 6 deletions src/department.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,17 @@ var Department = React.createClass({
{...this.props}
hideInput={true}
hideResults={true} />
<div className="departmentPage">
<DepartmentDecorator key={deptName} department={deptName} params={this.props.params} departmentInfo={this.props.data.departments} />
</div>
<Peek facet="department" q={deptName} quiltProps={{maxRowHeight: 400}} offset={10}/>
<Helmet title={deptName} />
<Departments compact={true} active={deptName} />
<main role="main" className="departmentPage" id="main">

@nlaffey nlaffey Dec 27, 2019 •
edited
Loading

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

By using the main element we help tell screenreaders/search engines where the primary content is. This can help consumers more quickly locate the unique content of this page rather than scaffolding around it (footer, navigation, etc).

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/main
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles

<DepartmentDecorator
key={deptName}
department={deptName}
params={this.props.params}
departmentInfo={this.props.data.departments}
/>
<Peek facet="department" q={deptName} quiltProps={{ maxRowHeight: 400 }} offset={10} />
<Helmet title={deptName} />
<Departments compact={true} active={deptName} />
</main>
</div>
}
})
Expand Down
4 changes: 2 additions & 2 deletions src/departments.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var Departments = React.createClass({

return (
<div className="departmentList mdl-grid" id="departments">
<h2 style={this.props.style}>Departments</h2>
<h2 style={this.props.style} role="heading" aria-level={1}>Departments</h2>
{this.departments.map(dept => {
var name = index => findDepartment(dept)[index]
var isActive = active === dept
Expand All @@ -52,7 +52,7 @@ var Departments = React.createClass({
key={name(1)}
params={{ dept: name(2) }}
className={`departmentLink mdl-cell mdl-cell--3-col ${isActive &&
'isActive'}`}
'isActive'}`}
>
<div className={[name(1), 'departmentListItem'].join(' ')} />
<h2 style={this.props.style}>{dept}</h2>
Expand Down
4 changes: 2 additions & 2 deletions src/home.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ var HomeDepartmentsAndPages = React.createClass({
pages: ['Purcell-Cutts House', 'Provenance Research', 'Deaccessions', 'Conservation'],

render() {
return <div className="landingPageBody">
return <main role="main" id="main" className="landingPageBody">

@nlaffey nlaffey Dec 27, 2019 •
edited
Loading

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Same concept as here: https://github.com/nlaffey/art/pull/2/files#r361749695

There's also likely other routes on the site that could use this same treatment.

<div className="mdl-grid">
<ul className="info">
<li className="mdl-cell mdl-cell--4-col">
Expand Down Expand Up @@ -97,6 +97,6 @@ var HomeDepartmentsAndPages = React.createClass({
})}
</ul>
</div>
</div>
</main>
},
})
16 changes: 13 additions & 3 deletions src/image-quilt.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ var linearPartition = require('linear-partitioning')
var ArtworkImage = require('./artwork-image')
var Image = require('./image')

const ENTER_KEY_CODE = 13;

const ImageQuilt = React.createClass({
mixins: [PureRenderMixin],

Expand Down Expand Up @@ -38,7 +40,6 @@ const ImageQuilt = React.createClass({
componentWillUnmount: function() {
window.removeEventListener('resize', this.handleResize)
},

render() {
if(this.hideDarkenedQuilt()) return this.emptyQuiltToggleControl()

Expand Down Expand Up @@ -89,7 +90,7 @@ const ImageQuilt = React.createClass({
onImageInvalidation={this.forceUpdate.bind(this)}
key={_art.id}
lazyLoad={this.props.lazyLoad}
/>
/>
})

// centered doesn't work on the first row because the search box is in the way
Expand Down Expand Up @@ -207,7 +208,7 @@ module.exports = ImageQuilt

var QuiltPatch = React.createClass({
render() {
var {art, width, ...other} = this.props
var {art, width, onClick, ...other} = this.props
var id = art.id

var style = {
Expand All @@ -228,6 +229,7 @@ var QuiltPatch = React.createClass({
}

var image = <Image
onClick={onClick}
art={art}
style={imgStyle}
{...other}
Expand All @@ -243,7 +245,14 @@ var QuiltPatch = React.createClass({
<p><strong>{art.title_short}</strong></p>
</span>

const handleKeyDown = function(event){
if(event.keyCode === ENTER_KEY_CODE){
onClick()
}
};

return <Link
onKeyDown={handleKeyDown}

@nlaffey nlaffey Dec 27, 2019

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Enter key presses were not registering as clicks. You could tab to the image but you weren't able to access it without using the mouse. This allows the enter key to be pressed.

onClick={this.clickOrDontClick}
style={{...style, position: 'relative'}}
to="artwork"
Expand All @@ -256,6 +265,7 @@ var QuiltPatch = React.createClass({
if(!this.context.universal) event.preventDefault()
},


getDefaultProps() {
return {
lazyLoad: true,
Expand Down
2 changes: 2 additions & 0 deletions src/map.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ var Map = React.createClass({
render() {
var {number, prevLink, nextLink, ...extraProps} = this.props
var thumbnail = <img
// TODO: Update to pass in an appropriate description of the image instead of just the number

@nlaffey nlaffey Dec 27, 2019

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Unfortunately there didn't seem to be a quick way to get the alt text here. Generally I feel that some alt text is better than nothing.

alt={`Image number ${number} from MIA gallery`}
src={`https://artsmia.github.io/map/galleries/${number}.png`}
onClick={this.handleClick}
/>
Expand Down
4 changes: 2 additions & 2 deletions src/navigation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var GlobalNavigation = React.createClass({

if(smallViewport){
return (
<div className='mobile-nav nav_bar' onClick={this.closeNav}>
<nav role="navigation" className='mobile-nav nav_bar' onClick={this.closeNav}>

<div className="global_nav">
<ul className="nav_list">
Expand All @@ -48,7 +48,7 @@ var GlobalNavigation = React.createClass({
})}
</ul>
</div>
</div>
</nav>
)
} else {
return (
Expand Down
14 changes: 10 additions & 4 deletions src/rights-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,14 @@ const publicDomainMark = {
definition: 'This work has been identified as being free of known restrictions under copyright law, including all related and neighboring rights.\n\nYou can copy, modify, distribute and perform the work, even for commercial purposes, all without asking permission. See Other Information below.',
}

// TODO: It appears that this module is installed locally. For now I'm just going to comment this out and replace it
// with an empty array so I can get to the accessibility fixes.
// const importedRightsStatements = require('rightsstatements');

const importedRightsStatements = [];

const rightsstatements = [
...require('rightsstatements').map(rs => {
...importedRightsStatements.map(rs => {
rs.definition = rs.definition
.replace('The copyright and related rights status of this Item has not been evaluated', 'The copyright and related rights status of this Item has not been evaluated by the Minneapolis Institute of Art (Mia)')
.replace('Please refer to the organization that has made the Item available for more information.', 'Please contact us for more information.')
Expand Down Expand Up @@ -46,7 +52,7 @@ const legacyToStandardizedStmt = {
"Full Permission": "In Copyright - Non-Commercial Use Permitted",
"Orphaned Work": "In Copyright - Rights-holder(s) Unlocatable or Unidentifiable",
"Permission Denied": "In Copyright", // TODO what goes here? Empty in Heidi's mapping
"Public Domain": "Public Domain",
"Public Domain": "Public Domain",
"No Known Copyright Restrictions": "No Known Copyright",
"Need Permission": "Copyright Not Evaluated",
}
Expand Down Expand Up @@ -97,10 +103,10 @@ function RightsStatementIcon({statement, color, style, ...props}) {
if(!statement) return <span />

var shortId = statement.identifier.split('-')[0]
var iconURL = statement.identifier === 'CC-PDM' ?
var iconURL = statement.identifier === 'CC-PDM' ?
`https://upload.wikimedia.org/wikipedia/commons/a/af/Cc-public_domain_mark.svg`
: `https://rightsstatements.org/files/icons/${shortId}.Icon-Only.${color || 'white'}.svg`
var fullIconURL = statement.identifier === 'CC-PDM' ?
var fullIconURL = statement.identifier === 'CC-PDM' ?
`https://upload.wikimedia.org/wikipedia/commons/a/af/Cc-public_domain_mark.svg`
: `https://rightsstatements.org/files/buttons/${statement.identifier}.${color || 'white'}.svg`

Expand Down
Loading