Skip to content

Commit

Permalink
fix: sd-map-marker a11y (#1577)
Browse files Browse the repository at this point in the history
<!-- ## Title: Please consider adding the [skip chromatic] flag to the
PR title in case you dont need chromatic testing your changes. -->
## Description:
This PR closes
[1480](#1480) and
[1588](#1588)

## Definition of Reviewable:
<!-- *PR notes: Irrelevant elements should be removed.* -->
- [x] Documentation is created/updated
- [ ] Migration Guide is created/updated
- [x] E2E tests (features, a11y, bug fixes) are created/updated
<!-- *If this PR includes a bug fix, an E2E test is necessary to verify
the change. If the fix is purely visual, ensuring it is captured within
our chromatic screenshot tests is sufficient.* -->
- [x] Stories (features, a11y) are created/updated
- [x] relevant tickets are linked
  • Loading branch information
smfonseca authored Oct 31, 2024
1 parent 1ed97a5 commit d755c32
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
import '../../solid-components';
import { html } from 'lit-html';
import { storybookDefaults, storybookTemplate } from '../../../scripts/storybook/helper';
import { storybookDefaults, storybookHelpers, storybookTemplate } from '../../../scripts/storybook/helper';
import { withActions } from '@storybook/addon-actions/decorator';

const { argTypes, parameters } = storybookDefaults('sd-map-marker');
const { overrideArgs } = storybookHelpers('sd-map-marker');
const { generateTemplate } = storybookTemplate('sd-map-marker');

/**
* Used to show a location or a cluster of locations on a map.
*
* **Accessibility Hint**: if interactive, make sure to provide an accessible name by adding a descriptive text to the `default` slot and visually hide it if needed.
*
* **Related templates:**
* - [Map Marker](?path=/docs/templates-map-marker--docs)
*/
export default {
title: 'Components/sd-map-marker',
tags: ['!dev'],
component: 'sd-map-marker',
args: overrideArgs([{ type: 'slot', name: 'default', value: '<div class="sr-only">Pinned Location</div>' }]),
parameters: {
...parameters,
design: {
Expand Down Expand Up @@ -43,11 +47,17 @@ export const Default = {
export const Variant = {
render: () => html`
<div class="flex items-center gap-12">
<sd-map-marker></sd-map-marker>
<sd-map-marker>
<span class="sr-only">Pinned Location</span>
</sd-map-marker>
<sd-map-marker variant="place">
<sd-icon name="content/image" color="primary"></sd-icon>
<span class="sr-only">Pinned Place</span>
</sd-map-marker>
<sd-map-marker variant="cluster">
88
<span class="sr-only">Locations</span>
</sd-map-marker>
<sd-map-marker variant="cluster"> 88 </sd-map-marker>
</div>
`
};
Expand All @@ -63,26 +73,41 @@ export const State = {
render: () => html`
<div class="flex gap-12">
<div class="flex flex-col space-y-5">
<sd-map-marker state="default"></sd-map-marker>
<sd-map-marker state="hover"></sd-map-marker>
<sd-map-marker state="active"></sd-map-marker>
<sd-map-marker state="default">
<span class="sr-only">Pinned location with default state</span>
</sd-map-marker>
<sd-map-marker state="hover">
<span class="sr-only">Pinned location with hover state</span>
</sd-map-marker>
<sd-map-marker state="active">
<span class="sr-only">Pinned location with active state</span>
</sd-map-marker>
</div>
<div class="flex flex-col space-y-5">
<sd-map-marker state="default" variant="place">
<sd-icon name="content/image" color="primary"></sd-icon>
<span class="sr-only">Pinned place with default state</span>
</sd-map-marker>
<sd-map-marker state="hover" variant="place">
<sd-icon name="content/image" color="primary"></sd-icon>
<span class="sr-only">Pinned place with hover state</span>
</sd-map-marker>
<sd-map-marker state="active" variant="place">
<sd-icon name="content/image" color="primary"></sd-icon>
<span class="sr-only">Pinned place with active state</span>
</sd-map-marker>
</div>
<div class="flex flex-col space-y-5">
<sd-map-marker state="default" variant="cluster"> 88 </sd-map-marker>
<sd-map-marker state="hover" variant="cluster"> 88 </sd-map-marker>
<sd-map-marker state="default" variant="cluster">
<span class="sr-only">Cluster of locations</span>
88
</sd-map-marker>
<sd-map-marker state="hover" variant="cluster">
<span class="sr-only">Hovered Cluster of locations</span>
88
</sd-map-marker>
</div>
</div>
`
Expand All @@ -93,7 +118,15 @@ export const State = {
*/
export const Animated = {
render: () => html`
<sd-map-marker class="animated-example" variant="main" state="default" animated=""></sd-map-marker>
<div class="flex items-center gap-12">
<sd-map-marker
class="animated-example"
variant="main"
state="default"
animated=""
not-interactive
></sd-map-marker>
</div>
<script>
const marker = document.querySelector('.animated-example');
setInterval(() => {
Expand All @@ -110,6 +143,39 @@ export const Slot = {
render: () => html`
<sd-map-marker variant="place" class="slot-example">
<span class="slot slot--border h-8 w-8 -mt-4"></span>
<div class="sr-only">Pinned Place with slot</div>
</sd-map-marker>
`
};

/**
* Use the `not-interactive` attribute to render a marker that is not interactive.
*/
export const NotInteractive = {
render: () => html`
<div class="flex items-center gap-12">
<sd-map-marker not-interactive></sd-map-marker>
<sd-map-marker variant="place" not-interactive>
<sd-icon name="content/image" color="primary"></sd-icon>
</sd-map-marker>
<sd-map-marker variant="cluster" not-interactive> 88 </sd-map-marker>
</div>
`
};

/**
* - Use the `href` attribute to enable the link.
* - Use the `target` attribute to specify where to open the link.
*/
export const AsLink = {
render: () => html`
<div class="flex items-center gap-12">
<sd-map-marker
href="https://solid-design-system.fe.union-investment.de/x.x.x/storybook/?path=/docs/docs-general-introduction--docs"
target="_blank"
>
<span class="sr-only">Solid Design System by Union Investment</span>
</sd-map-marker>
</div>
`
};
29 changes: 27 additions & 2 deletions packages/components/src/components/map-marker/map-marker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,19 @@ import { expect, fixture } from '@open-wc/testing';
import { html } from 'lit-html';
import type SdMapMarker from './map-marker';

describe('<sd-navigation-item>', () => {
describe('<sd-map-marker>', () => {
// Test default button variant
describe('by default', () => {
// Accessibility
it('passes accessibility test', async () => {
const el = await fixture<SdMapMarker>(html` <sd-map-marker></sd-map-marker>`);
const el = await fixture<SdMapMarker>(html` <sd-map-marker not-interactive></sd-map-marker>`);
await expect(el).to.be.accessible();
});

it('passes accessibility test when it renders a button', async () => {
const el = await fixture<SdMapMarker>(
html` <sd-map-marker><div class="sr-only">Acessible Pin</div></sd-map-marker>`
);
await expect(el).to.be.accessible();
});

Expand All @@ -19,6 +26,7 @@ describe('<sd-navigation-item>', () => {
expect(el.animated).to.equal(false);
});
});

describe('svgs have a width bigger than 1 in all browsers', () => {
it('for main variant', async () => {
const el = await fixture<SdMapMarker>(html` <sd-map-marker></sd-map-marker> `);
Expand All @@ -33,4 +41,21 @@ describe('<sd-navigation-item>', () => {
expect(el.shadowRoot!.querySelector('svg')?.getBoundingClientRect().width).to.be.greaterThan(0);
});
});

describe('renders a different tag according to the not-interactive attribute', () => {
it('when not interactive renders a div', async () => {
const el = await fixture<SdMapMarker>(html` <sd-map-marker not-interactive></sd-map-marker> `);
expect(el.shadowRoot!.querySelector('div')).to.exist;
});

it('when interactive but without an href, renders a button', async () => {
const el = await fixture<SdMapMarker>(html` <sd-map-marker></sd-map-marker> `);
expect(el.shadowRoot!.querySelector('button')).to.exist;
});

it('when interactive and href is set', async () => {
const el = await fixture<SdMapMarker>(html` <sd-map-marker href="https://www.solid.com"></sd-map-marker> `);
expect(el.shadowRoot!.querySelector('a')).to.exist;
});
});
});
64 changes: 56 additions & 8 deletions packages/components/src/components/map-marker/map-marker.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { css, html, svg } from 'lit';
import { css, svg } from 'lit';
import { customElement } from '../../../src/internal/register-custom-element';
import { html, literal } from 'lit/static-html.js';
import { ifDefined } from 'lit/directives/if-defined.js';
import { property } from 'lit/decorators.js';
import componentStyles from '../../styles/component.styles';
import cx from 'classix';
Expand All @@ -13,7 +15,10 @@ import type { SVGTemplateResult } from 'lit';
* @status stable
* @since 2.12
*
* @slot - The marker's content.
* @slot - The marker's content.\
*
* @event sd-blur - Emitted when the map marker loses focus.
* @event sd-focus - Emitted when the map marker is focused.
*
* @cssproperty --map-marker-scaling - Scale the marker size.
*/
Expand All @@ -28,6 +33,27 @@ export default class SdMapMarker extends SolidElement {
/** The map-marker's is animated when displayed. */
@property({ type: Boolean, reflect: true }) animated = false;

/** Determines if the map-marker is interactive. */
@property({ type: Boolean, reflect: true, attribute: 'not-interactive' }) notInteractive = false;

/** When set, the underlying button will be rendered as an `<a>` with this `href` instead of a `<button>`. */
@property() href = '';

/** Tells the browser where to open the link. Only used when `href` is present. */
@property() target: '_blank' | '_parent' | '_self' | '_top';

private handleBlur() {
this.emit('sd-blur');
}

private handleFocus() {
this.emit('sd-focus');
}

private isLink() {
return this.href ? true : false;
}

/** @internal */
readonly marker: { [key in 'cluster' | 'main' | 'place']: SVGTemplateResult } = {
cluster: svg`<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 50 50">
Expand All @@ -41,35 +67,57 @@ export default class SdMapMarker extends SolidElement {
};

render() {
const isLink = this.isLink();
const tag = this.notInteractive ? literal`div` : isLink ? literal`a` : literal`button`;

/* eslint-disable lit/no-invalid-html */
/* eslint-disable lit/binding-positions */
return html`
<div part="base" tabindex="0" class="flex justify-center focus:outline-primary focus:outline-offset-2">
<${tag}
part="base"
class=${cx(
'flex justify-center',
!this.notInteractive && 'focus:outline focus:outline-2 focus:outline-primary focus:outline-offset-2'
)}
href=${ifDefined(isLink ? this.href : undefined)}
target=${ifDefined(isLink ? this.target : undefined)}
@blur=${this.handleBlur}
@focus=${this.handleFocus}
aria-labelledby="content"
>
<div
part="marker"
class=${cx(
'inline-flex',
this.animated && (this.variant === 'main' || this.variant === 'place') && 'animate-bounce-once',
this.variant === 'cluster' && this.state === 'hover' && 'scale-110 fill-primary-500',
this.variant === 'cluster' &&
!this.notInteractive &&
'transition-all duration-200 ease-in-out hover:scale-110 hover:fill-primary-500',
this.variant === 'main' && this.state === 'hover' && 'fill-accent-550',
this.variant === 'main' && this.state === 'active' && 'fill-accent-700',
this.variant === 'main' && !this.notInteractive && 'hover:fill-accent-550 active:fill-accent-700',
this.variant === 'place' && this.state === 'default' && 'fill-white',
this.variant === 'place' && this.state === 'hover' && 'fill-primary-100',
this.variant === 'place' && this.state === 'active' && 'fill-primary-200',
this.variant === 'place' && !this.notInteractive && 'hover:fill-primary-100',
{
cluster: 'fill-primary transition-all duration-200 ease-in-out hover:scale-110 hover:fill-primary-500',
main: 'fill-accent *:drop-shadow-md hover:fill-accent-550 active:fill-accent-700',
place: 'hover:fill-primary-100 *:drop-shadow-md'
cluster: 'fill-primary',
main: 'fill-accent *:drop-shadow-md',
place: '*:drop-shadow-md'
}[this.variant]
)}
>
${this.marker[this.variant]}
</div>
<div
id="content"
part="content"
class=${cx('absolute self-center pointer-events-none', this.variant === 'cluster' && 'text-white')}
class=${cx('absolute self-center pointer-events-none', this.variant === 'cluster' && 'font-bold text-white')}
>
<slot></slot>
</div>
</div>
</${tag}>
`;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/templates/map-marker.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ export default {
};

/**
* ### Map Maker with Images
* ### Map Marker with Image
*/
export const MapMakerWithImages = {
render: () =>
html`<sd-map-marker state="default" variant="place">
html`<sd-map-marker state="default" variant="place" label="Union Investment location" not-interactive>
<img src="images/ui-brand-mark.png" alt="Brand mark of Union Investment" class="h-8 w-8 -mt-4" />
</sd-map-marker>`
};

0 comments on commit d755c32

Please sign in to comment.