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.

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

Jump to bottom

Create <map-caption> custom element #741

Merged
merged 15 commits into from
Feb 15, 2023
Merged

Create <map-caption> custom element #741

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
6fc0d56
implement <map-caption> custom element
kevinkim31 Feb 3, 2023
2f8c5f7
Add caption to demo map that is distributed with dist
Feb 7, 2023
2c4b1f9
remove delete function for now, add aria-label precheck
kevinkim31 Feb 8, 2023
01c525b
Merge branch 'map-caption' of https://github.com/kevinkim31/Web-Map-C…
kevinkim31 Feb 8, 2023
7320940
multiple arialabels
kevinkim31 Feb 10, 2023
8ea7b40
major fixes
kevinkim31 Feb 10, 2023
d7967cf
add testing and slight modifications to further bulletproof
kevinkim31 Feb 10, 2023
555e817
change document -> this
kevinkim31 Feb 10, 2023
4a5f24e
syntax fixing
kevinkim31 Feb 13, 2023
06bbf29
test
Feb 13, 2023
73628cd
test 2
kevinkim31 Feb 13, 2023
dc50104
test 3
kevinkim31 Feb 13, 2023
fb03226
add testing for layer and other fixes
kevinkim31 Feb 14, 2023
a2bfa4d
Merge branch 'map-caption' of https://github.com/kevinkim31/Web-Map-C…
kevinkim31 Feb 14, 2023
e252ac2
move function inside isconnected block
kevinkim31 Feb 14, 2023
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 Gruntfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module.exports = function(grunt) {
'dist/mapml.js': ['<%= rollup.main.dest %>'],
'dist/web-map.js': ['src/web-map.js'],
'dist/mapml-viewer.js': ['src/mapml-viewer.js'],
'dist/map-caption.js': ['src/map-caption.js'],
'dist/map-area.js': ['src/map-area.js'],
'dist/layer.js': ['src/layer.js'],
'dist/leaflet.js': ['dist/leaflet-src.js',
Expand Down
1 change: 1 addition & 0 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
</head>
<body>
<mapml-viewer projection="CBMTILE" zoom="2" lat="45" lon="-90" controls>
<map-caption>A pleasing map of Canada</map-caption>
<layer- label="CBMT" src="https://geogratis.gc.ca/mapml/en/cbmtile/cbmt/" checked></layer->
</mapml-viewer>
</body>
Expand Down
45 changes: 45 additions & 0 deletions src/map-caption.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import './leaflet.js';
kevinkim31 marked this conversation as resolved.
Show resolved Hide resolved
import './mapml.js';

/*
implemented for both mapml-viewer and web-map; however web-map does not focus on map element in the browser resulting in NVDA
not being able to read out map-caption and stating that it's an interactive map region
*/
export class MapCaption extends HTMLElement {
constructor() {
super();
}

// called when element is inserted into DOM (setup code)
connectedCallback() {

kevinkim31 marked this conversation as resolved.
Show resolved Hide resolved
// calls MutationObserver; needed to observe changes to content between <map-caption> tags and update to aria-label
let mapcaption = this.parentElement.querySelector('map-caption').innerHTML;
Copy link
Member

Choose a reason for hiding this comment

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

I think you can use this.textContent here, unless I'm mistaken. Also, the query you're making is returning the first and only the first of potentially many <map-caption> elements, which will sometimes be this one, and sometimes not.

I think .textContent is appropriate here. I could be convinced otherwise, but See for a discussion: https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent#differences_from_innertext

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For mapcaption, I need this to return the first map-caption only so that I can store it to then compare it to the mapcaption that was removed/modified.


this.observer = new MutationObserver(() => {
let mapcaptionupdate = this.parentElement.querySelector('map-caption').innerHTML;

if (mapcaptionupdate != mapcaption)
kevinkim31 marked this conversation as resolved.
Show resolved Hide resolved
{
kevinkim31 marked this conversation as resolved.
Show resolved Hide resolved
this.parentElement.setAttribute('aria-label', this.parentElement.querySelector('map-caption').textContent);
}
});

this.observer.observe(this, {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe observing too much, perhaps only subtree is necessary?

Copy link
Member

Choose a reason for hiding this comment

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

Still think that we're observing too much for <map-caption>, only need to watch our content / subtree, no? If you disagree, put a comment below, then mark as resolved. Thanks.

characterData: true,
subtree: true,
attributes: true,
childList: true
});

// don't change aria-label if one already exists from user (checks when element is first created)
if (!this.parentElement.hasAttribute('aria-label'))
{
const ariaLabel = this.textContent;
Copy link
Member

Choose a reason for hiding this comment

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

Good use of textContent, see comment above

this.parentElement.setAttribute('aria-label', ariaLabel);
}
}
disconnectedCallback() {
this.observer.disconnect();
}
}
32 changes: 32 additions & 0 deletions src/mapml-viewer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import './leaflet.js'; // bundled with proj4, proj4leaflet, modularized
import './mapml.js';
import { MapLayer } from './layer.js';
import { MapCaption } from './map-caption.js';

export class MapViewer extends HTMLElement {
static get observedAttributes() {
Expand Down Expand Up @@ -234,6 +235,36 @@ export class MapViewer extends HTMLElement {
this.dispatchEvent(new CustomEvent('createmap'));
}
}

/*
1. only deletes aria-label when the last (only remaining) map caption is removed
2. only deletes aria-label if the aria-label was defined by the map caption element itself
kevinkim31 marked this conversation as resolved.
Show resolved Hide resolved
*/

let mapcaption = this.querySelector('map-caption');

if (mapcaption != null){
setTimeout(() => {
let ariaupdate = this.getAttribute('aria-label');

if (ariaupdate == mapcaption.innerHTML) {
this.mapCaptionObserver = new MutationObserver((m) => {
let mapcaptionupdate = this.querySelector('map-caption');
if (mapcaptionupdate != mapcaption)
{
this.removeAttribute('aria-label');
}

});
this.mapCaptionObserver.observe(this, {
characterData: true,
subtree: true,
attributes: true,
childList: true
});
}
}, 0);
}
}
disconnectedCallback() {
//this._removeEvents();
Expand Down Expand Up @@ -783,3 +814,4 @@ export class MapViewer extends HTMLElement {
// need to provide options { extends: ... } for custom built-in elements
window.customElements.define('mapml-viewer', MapViewer);
window.customElements.define('layer-', MapLayer);
window.customElements.define('map-caption',MapCaption);
32 changes: 32 additions & 0 deletions src/web-map.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import './leaflet.js'; // a lightly modified version of Leaflet for use as brow
import './mapml.js'; // refactored URI usage, replaced with URL standard
import { MapLayer } from './layer.js';
import { MapArea } from './map-area.js';
import { MapCaption } from './map-caption.js';

export class WebMap extends HTMLMapElement {
static get observedAttributes() {
Expand Down Expand Up @@ -275,6 +276,36 @@ export class WebMap extends HTMLMapElement {
this.dispatchEvent(new CustomEvent('createmap'));
}
}

/*
1. only deletes aria-label when the last (only remaining) map caption is removed
2. only deletes aria-label if the aria-label was defined by the map caption element itself
*/

let mapcaption = this.querySelector('map-caption');

if (mapcaption != null){
setTimeout(() => {
Copy link
Member

Choose a reason for hiding this comment

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

Why is it required to put this on the event queue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is so that the ariaupdate has time to load

let ariaupdate = this.getAttribute('aria-label');

if (ariaupdate == mapcaption.innerHTML) {
kevinkim31 marked this conversation as resolved.
Show resolved Hide resolved
this.mapCaptionObserver = new MutationObserver((m) => {
let mapcaptionupdate = this.querySelector('map-caption');
if (mapcaptionupdate != mapcaption)
kevinkim31 marked this conversation as resolved.
Show resolved Hide resolved
{
this.removeAttribute('aria-label');
}

});
this.mapCaptionObserver.observe(this, {
characterData: true,
subtree: true,
attributes: true,
kevinkim31 marked this conversation as resolved.
Show resolved Hide resolved
childList: true
});
}
}, 0);
}
}
disconnectedCallback() {
//this._removeEvents();
Expand Down Expand Up @@ -844,3 +875,4 @@ export class WebMap extends HTMLMapElement {
window.customElements.define('web-map', WebMap, { extends: 'map' });
window.customElements.define('layer-', MapLayer);
window.customElements.define('map-area', MapArea, {extends: 'area'});
window.customElements.define('map-caption',MapCaption);
33 changes: 33 additions & 0 deletions test/e2e/mapml-viewer/mapml-viewerCaption.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="UTF-8">

<script type="module" src="mapml-viewer.js"></script>
<style>
html {
height: 100%
}

body {
height: inherit
}

* {
margin: 0;
padding: 0;
}
</style>
</head>

<body>

<mapml-viewer id="nocontrols" style="height: 600px;width:500px;" projection="CBMTILE" zoom="0" lat="47" lon="-92">
<map-caption>This is a test for mapml-viewer</map-caption>
<map-caption id="test2">Test2</map-caption>
<map-caption id="test3">Test3</map-caption>
</mapml-viewer>

</body>
</html>
43 changes: 43 additions & 0 deletions test/e2e/mapml-viewer/mapml-viewerCaption.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { test, expect, chromium } from '@playwright/test';

test.describe("Playwright mapml-viewer map-captions Test", () => {
let page;
let context;
test.beforeAll(async () => {
context = await chromium.launchPersistentContext('');
page = context.pages().find((page) => page.url() === 'about:blank') || await context.newPage();
page = await context.newPage();
await page.goto("mapml-viewerCaption.html");
});

test.afterAll(async function () {
await context.close();
});

test("Aria-label matches map-caption", async () => {
let arialabel = await page.evaluate(`document.querySelector('mapml-viewer').getAttribute('aria-label')`);
expect(arialabel).toEqual("This is a test for mapml-viewer");
});
test("Changing first map-caption changes aria-label", async () => {
await page.evaluateHandle(() => document.querySelector('map-caption').innerHTML="Testing 1");
let arialabel = await page.evaluate(`document.querySelector('mapml-viewer').getAttribute('aria-label')`);
expect(arialabel).toEqual("Testing 1");
});
test("Changing not-first map-caption doesn't change aria-label", async () => {
await page.evaluateHandle(() => document.getElementById('test2').innerHTML="Testing 2");
let arialabel = await page.evaluate(`document.querySelector('mapml-viewer').getAttribute('aria-label')`);
expect(arialabel).toEqual("Testing 1"); // since aria-label didn't change, should still = "Testing 1" from previous test
});
test("Removing not-first map-caption doesn't remove aria-label", async () => {
await page.evaluateHandle(() => document.getElementById('test3').remove());
let arialabel = await page.evaluate(`document.querySelector('mapml-viewer').getAttribute('aria-label')`);
expect(arialabel).toEqual("Testing 1"); // since aria-label is still there, shoudl still = "Testing 1" from previous test
});
test("Removing first map-caption removes aria-label", async () => {
await page.evaluateHandle(() => document.querySelector('map-caption').remove());
let arialabel = await page.evaluate(`document.querySelector('mapml-viewer').getAttribute('aria-label')`);
expect(arialabel).toEqual(null); // since aria-label is removed, should = null
});


});
33 changes: 33 additions & 0 deletions test/e2e/web-map/mapCaption.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="UTF-8">

<script type="module" src="web-map.js"></script>
<style>
html {
height: 100%
}

body {
height: inherit
}

* {
margin: 0;
padding: 0;
}
</style>
</head>

<body>

<map is="web-map" style="width: 500px;height: 500px;" projection="CBMTILE" zoom="2" lat="45.5052040" lon="-75.2202344">
<map-caption>This is a test for web-map</map-caption>
<map-caption id="test2">Test2</map-caption>
<map-caption id="test3">Test3</map-caption>
</map>

</body>
</html>
42 changes: 42 additions & 0 deletions test/e2e/web-map/mapCaption.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { test, expect, chromium } from '@playwright/test';

test.describe("Playwright web-map map-captions Test", () => {
let page;
let context;
test.beforeAll(async () => {
context = await chromium.launchPersistentContext('');
page = context.pages().find((page) => page.url() === 'about:blank') || await context.newPage();
page = await context.newPage();
await page.goto("mapCaption.html");
});

test.afterAll(async function () {
await context.close();
});

test("Aria-label matches map-caption", async () => {
let arialabel = await page.evaluate(`document.querySelector('map').getAttribute('aria-label')`);
expect(arialabel).toEqual("This is a test for web-map");
});
test("Changing map-caption changes aria-label", async () => {
await page.evaluateHandle(() => document.querySelector('map-caption').innerHTML="Testing 1");
let arialabel = await page.evaluate(`document.querySelector('map').getAttribute('aria-label')`);
expect(arialabel).toEqual("Testing 1");
});
test("Changing not-first map-caption doesn't change aria-label", async () => {
await page.evaluateHandle(() => document.getElementById('test2').innerHTML="Testing 2");
let arialabel = await page.evaluate(`document.querySelector('map').getAttribute('aria-label')`);
expect(arialabel).toEqual("Testing 1"); // since aria-label didn't change, should still = "Testing 1" from previous test
});
test("Removing not-first map-caption doesn't remove aria-label", async () => {
await page.evaluateHandle(() => document.getElementById('test3').remove());
let arialabel = await page.evaluate(`document.querySelector('map').getAttribute('aria-label')`);
expect(arialabel).toEqual("Testing 1"); // since aria-label is still there, shoudl still = "Testing 1" from previous test
});
test("Removing first map-caption removes aria-label", async () => {
await page.evaluateHandle(() => document.querySelector('map-caption').remove());
let arialabel = await page.evaluate(`document.querySelector('map').getAttribute('aria-label')`);
expect(arialabel).toEqual(null); // since aria-label is removed, should = null
});

});