Skip to content

Commit

Permalink
fix(link): review
Browse files Browse the repository at this point in the history
  • Loading branch information
aesteves60 authored and dpellier committed Jul 29, 2024
1 parent ff29c2f commit 72db99a
Show file tree
Hide file tree
Showing 17 changed files with 63 additions and 39 deletions.
2 changes: 1 addition & 1 deletion packages/ods/react/tests/e2e/ods-link.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('ods-link react', () => {
await goToComponentPage(page, 'ods-link');
});

it('render the component correctly', async () => {
it('should navigate on click', async () => {
const elem = await page.$('ods-link');

await elem?.click();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
## Attributes changes

`contrasted ` <img src="https://img.shields.io/badge/removed-FF0000" />
`solt ` <img src="https://img.shields.io/badge/updated-00FFFF" />
`slot ` <img src="https://img.shields.io/badge/updated-00FFFF" />

Have been removed.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
## Table of Contents
[• Properties](#properties)



[• Enums](#enums)

## Properties
### color
Expand Down Expand Up @@ -54,7 +48,7 @@


## Enums
## Enumeration: ODS_LINK_COLOR
### Enumeration: ODS_LINK_COLOR

**primary** = `"primary"`

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
:host(.ods-icon) {
<<<<<<< HEAD
/* use !important to prevent issues with browser extensions that change fonts */
line-height: 1;
font-family: "ODS Icon", serif !important;
Expand Down Expand Up @@ -608,4 +609,14 @@

:host(.ods-icon__warning-triangle)::before {
content: "\e995";
=======
display: inline-block;
-webkit-mask: center/contain no-repeat;
mask: center/contain no-repeat;
-webkit-mask-image: var(--ods-icon-mask-image, url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='32' height='32' viewBox='0 0 32 32'/%3E"));
mask-image: var(--ods-icon-mask-image, url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='32' height='32' viewBox='0 0 32 32'/%3E"));
background-color: var(--ods-color-primary-500);
width: 1rem;
height: 1rem;
>>>>>>> 923c5abac (fix(link): review)
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
import { Host, h } from "@stencil/core";
<<<<<<< HEAD
=======
import icons from "../../assets/icons.data.json"; // TODO replace with dedicated icon font when file server get available
>>>>>>> 923c5abac (fix(link): review)
export class OdsIcon {
constructor() {
this.alt = '';
this.name = undefined;
}
render() {
<<<<<<< HEAD
return (h(Host, { key: 'cd390c78bf5d9156308e947e7c45b3b44b0e7686', class: `ods-icon ods-icon__${this.name}`, alt: this.alt }));
=======
const base64Icon = icons[this.name];
return (h(Host, { key: '27d5d10767805aab9a2ac37006374103428f581e', class: "ods-icon", alt: this.alt, style: (base64Icon ? { '--ods-icon-mask-image': `url("${base64Icon}")` } : {}) }));
>>>>>>> 923c5abac (fix(link): review)
}
static get is() { return "ods-icon"; }
static get encapsulation() { return "shadow"; }
Expand Down Expand Up @@ -44,7 +53,11 @@ export class OdsIcon {
"mutable": false,
"complexType": {
"original": "OdsIconName",
<<<<<<< HEAD
"resolved": "\"apps\" | \"arrow-crossed\" | \"arrow-down-left\" | \"arrow-down-right\" | \"arrow-down\" | \"arrow-left-right\" | \"arrow-left\" | \"arrow-right\" | \"arrow-up-down\" | \"arrow-up-left\" | \"arrow-up-right\" | \"arrow-up\" | \"baremetal-rack\" | \"baremetal\" | \"bell\" | \"book\" | \"calendar\" | \"chat\" | \"check-circle\" | \"check-square\" | \"check\" | \"chevron-double-left\" | \"chevron-double-right\" | \"chevron-down\" | \"chevron-left\" | \"chevron-right\" | \"chevron-up\" | \"chrono\" | \"circle\" | \"clock-time-four\" | \"clock-time-nine\" | \"clock-time-six\" | \"clock-time-three\" | \"clock-time-twelve\" | \"cloud-check\" | \"cloud-download\" | \"cloud-lock\" | \"cloud-times\" | \"cloud-upload\" | \"cloud\" | \"cog\" | \"collab\" | \"component\" | \"credit-card\" | \"critical-hexagon-full\" | \"critical-hexagon\" | \"d-pad\" | \"danger-diamond-full\" | \"danger-diamond\" | \"desktop\" | \"diamond\" | \"dot-circle\" | \"download\" | \"ellipsis-horizontal\" | \"ellipsis-vertical\" | \"email\" | \"emoticon-dizzy\" | \"emoticon-neutral\" | \"emoticon-sad\" | \"emoticon-smile\" | \"emoticon-wink\" | \"emoticon\" | \"equal\" | \"error-circle\" | \"external-link\" | \"eye-off\" | \"eye\" | \"file-copy\" | \"file-minus\" | \"file-plus\" | \"file\" | \"filter\" | \"focus\" | \"folder-minus\" | \"folder-plus\" | \"folder\" | \"game-console\" | \"game-controller-alt\" | \"game-controller\" | \"gathering\" | \"globe\" | \"grid\" | \"hamburger-menu\" | \"hexagon\" | \"hierarchy\" | \"home\" | \"info-circle\" | \"key\" | \"labs-empty\" | \"labs-full\" | \"labs-half\" | \"leaf\" | \"lifebuoy\" | \"lightbulb\" | \"list\" | \"location\" | \"lock-close\" | \"lock-open\" | \"minus-square\" | \"minus\" | \"money\" | \"network\" | \"pen\" | \"phone\" | \"plus\" | \"printer\" | \"product-3az\" | \"promotion\" | \"question-circle\" | \"question\" | \"radio\" | \"refresh\" | \"resize\" | \"search\" | \"share\" | \"shield-check\" | \"shield-firewall\" | \"shield-lock\" | \"shield-minus\" | \"shield-off\" | \"shield-plus\" | \"shield-times\" | \"shield-warning\" | \"shield\" | \"shopping-cart-clear\" | \"shopping-cart-error\" | \"shopping-cart-minus\" | \"shopping-cart-plus\" | \"shopping-cart\" | \"shrink\" | \"shutdown\" | \"sort-alpha-down\" | \"sort-alpha-up\" | \"sort-numeric-down\" | \"sort-numeric-up\" | \"square\" | \"star-full\" | \"star\" | \"store\" | \"times\" | \"traffic-cone\" | \"trash\" | \"triangle\" | \"truck\" | \"undo\" | \"upload\" | \"user-circle\" | \"user\" | \"warning-triangle-full\" | \"warning-triangle\"",
=======
"resolved": "\"arrow-left\" | \"warning\"",
>>>>>>> 923c5abac (fix(link): review)
"references": {
"OdsIconName": {
"location": "import",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
var ODS_ICON_NAME;
(function (ODS_ICON_NAME) {
<<<<<<< HEAD
ODS_ICON_NAME["apps"] = "apps";
ODS_ICON_NAME["arrowCrossed"] = "arrow-crossed";
ODS_ICON_NAME["arrowDownLeft"] = "arrow-down-left";
Expand Down Expand Up @@ -150,6 +151,11 @@ var ODS_ICON_NAME;
ODS_ICON_NAME["user"] = "user";
ODS_ICON_NAME["warningTriangleFull"] = "warning-triangle-full";
ODS_ICON_NAME["warningTriangle"] = "warning-triangle";
=======
// TODO need actual list from design
ODS_ICON_NAME["arrowLeft"] = "arrow-left";
ODS_ICON_NAME["warning"] = "warning";
>>>>>>> 923c5abac (fix(link): review)
})(ODS_ICON_NAME || (ODS_ICON_NAME = {}));
const ODS_ICON_NAMES = Object.freeze(Object.values(ODS_ICON_NAME));
export { ODS_ICON_NAME, ODS_ICON_NAMES, };
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
&__link {
@include ods-link;

&-text {
&__text {
display: inline-flex;
align-items: center;
transition: background-size .2s ease-in, color ease-in-out .1s;
Expand All @@ -17,7 +17,7 @@

/* stylelint-disable-next-line order/order */
&:focus-visible, &:hover {
.ods-link__link-text {
.ods-link__link__text {
transition: background-size .2s ease-out;
background-size: 100% 0.1em;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ODS_LINK_COLOR, type OdsLinkColor } from '../../constant/link-color';
})
export class OdsLink {
@Prop({ reflect: true }) public color: OdsLinkColor = ODS_LINK_COLOR.primary;
@Prop({ reflect: true }) public disabled?: boolean;
@Prop({ reflect: true }) public disabled: boolean = false;
@Prop({ reflect: true }) public download?: HTMLAnchorElement['download'];
@Prop({ reflect: true }) public href!: string;
@Prop({ reflect: true }) public icon?: OdsIconName;
Expand All @@ -34,7 +34,7 @@ export class OdsLink {
rel={ this.rel }
tabindex={ this.disabled ? -1 : 0 }
target={ this.target }>
<span class="ods-link__link-text">
<span class="ods-link__link__text">
{ this.label }
</span>

Expand Down
4 changes: 4 additions & 0 deletions packages/ods/src/components/link/src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,9 @@
<p>Icon</p>
<ods-link href="test" label="link clickable" icon="warning">
</ods-link>

<p>Icon Disabled</p>
<ods-link href="test" label="link clickable" icon="warning" disabled>
</ods-link>
</body>
</html>
2 changes: 1 addition & 1 deletion packages/ods/src/components/link/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export { OdsLink } from './components/ods-link/ods-link';
export { ODS_LINK_COLOR, ODS_LINK_COLORS } from './constant/link-color';
export { ODS_LINK_COLOR, ODS_LINK_COLORS, type OdsLinkColor } from './constant/link-color';
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { E2EElement, E2EPage } from '@stencil/core/testing';
import { newE2EPage } from '@stencil/core/testing';

describe('ods-link accessibility', () => {
describe('ods-link navigation', () => {
let el: E2EElement;
let page: E2EPage;

Expand All @@ -21,14 +21,14 @@ describe('ods-link accessibility', () => {
expect(focusedTagName).toBe('ODS-LINK');
});

it('should not be focused with disabled', async() => {
it('should not be focused when disabled', async() => {
await setup('<ods-link label="some link" disabled href="https://www.ovhcloud.com/fr/"></ods-link>');
await page.keyboard.press('Tab');
const focusedTagName = await page.evaluate(() => document.activeElement?.tagName);
expect(focusedTagName).not.toBe('ODS-LINK');
});

it('should trigger link when Enter', async() => {
it('should trigger link on Enter', async() => {
const href = 'https://www.ovhcloud.com/fr/';
await setup(`<ods-link label="some link" href="${href}"></ods-link>`);
await page.keyboard.press('Tab');
Expand All @@ -37,7 +37,7 @@ describe('ods-link accessibility', () => {
expect(page.url()).toBe(href);
});

it('should not trigger link when Enter with disabled', async() => {
it('should not trigger link on Enter when disabled', async() => {
const href = 'https://www.ovhcloud.com/fr/';
await setup(`<ods-link label="some link" disabled href="${href}"></ods-link>`);
await page.keyboard.press('Tab');
Expand All @@ -46,15 +46,15 @@ describe('ods-link accessibility', () => {
expect(page.url()).not.toBe(href);
});

it('should trigger link when click', async() => {
it('should trigger link on click', async() => {
const href = 'https://www.ovhcloud.com/fr/';
await setup(`<ods-link label="some link" href="${href}"></ods-link>`);
await el.click();
await page.waitForNetworkIdle();
expect(page.url()).toBe(href);
});

it('should not trigger link when click with disabled', async() => {
it('should not trigger link on click when disabled', async() => {
const href = 'https://www.ovhcloud.com/fr/';
await setup(`<ods-link label="some link" disabled href="${href}"></ods-link>`);
await el.click();
Expand Down
22 changes: 10 additions & 12 deletions packages/ods/src/components/link/tests/rendering/ods-link.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('ods-link rendering', () => {
expect(root?.getAttribute('color')).toBe(colorValue);
});

it('should not be set by default', async() => {
it('should render with expected default value', async() => {
await setup(`<ods-link></ods-link>`);

expect(root?.getAttribute('color')).toBe( ODS_LINK_COLOR.primary);
Expand All @@ -35,14 +35,12 @@ describe('ods-link rendering', () => {

describe('disabled', () => {
it('should be reflected', async() => {
const disabledValue = true;

await setup(`<ods-link disabled="${disabledValue}"></ods-link>`);
await setup(`<ods-link disabled></ods-link>`);

expect(root?.getAttribute('disabled')).toBe('');
});

it('should not be set by default', async() => {
it('should render with expected default value', async() => {
await setup(`<ods-link></ods-link>`);

expect(root?.getAttribute('disabled')).toBe(null);
Expand All @@ -58,7 +56,7 @@ describe('ods-link rendering', () => {
expect(root?.getAttribute('download')).toBe(downloadValue);
});

it('should not be set by default', async() => {
it('should render with expected default value', async() => {
await setup(`<ods-link></ods-link>`);

expect(root?.getAttribute('download')).toBe(null);
Expand All @@ -74,7 +72,7 @@ describe('ods-link rendering', () => {
expect(root?.getAttribute('href')).toBe(hrefValue);
});

it('should not be set by default', async() => {
it('should render with expected default value', async() => {
await setup(`<ods-link></ods-link>`);

expect(root?.getAttribute('href')).toBe(null);
Expand All @@ -92,7 +90,7 @@ describe('ods-link rendering', () => {
expect(iconElement?.getAttribute('name')).toBe(iconValue);
});

it('should not be set by default', async() => {
it('should render with expected default value', async() => {
await setup(`<ods-link></ods-link>`);

expect(root?.getAttribute('icon')).toBe(null);
Expand All @@ -109,7 +107,7 @@ describe('ods-link rendering', () => {
expect(root?.getAttribute('label')).toBe(labelValue);
});

it('should not be set by default', async() => {
it('should render with expected default value', async() => {
await setup(`<ods-link></ods-link>`);

expect(root?.getAttribute('label')).toBe(null);
Expand All @@ -125,7 +123,7 @@ describe('ods-link rendering', () => {
expect(root?.getAttribute('referrerpolicy')).toBe(referrerpolicyValue);
});

it('should not be set by default', async() => {
it('should render with expected default value', async() => {
await setup(`<ods-link></ods-link>`);

expect(root?.getAttribute('referrerpolicy')).toBe(null);
Expand All @@ -141,7 +139,7 @@ describe('ods-link rendering', () => {
expect(root?.getAttribute('rel')).toBe(relValue);
});

it('should not be set by default', async() => {
it('should render with expected default value', async() => {
await setup(`<ods-link></ods-link>`);

expect(root?.getAttribute('rel')).toBe(null);
Expand All @@ -157,7 +155,7 @@ describe('ods-link rendering', () => {
expect(root?.getAttribute('target')).toBe(targetValue);
});

it('should not be set by default', async() => {
it('should render with expected default value', async() => {
await setup(`<ods-link></ods-link>`);

expect(root?.getAttribute('target')).toBe(null);
Expand Down
4 changes: 1 addition & 3 deletions packages/ods/src/style/_link.scss
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
@mixin ods-link() {
display: inline-flex;
align-items: center;
cursor: pointer;
text-decoration: none;
font-family: var(--ods-font-family-default);
font-weight: 600;
font-size: 16px;
line-height: 1;
cursor: pointer;

&:focus-visible {
outline-offset: 3px;
Expand Down
2 changes: 1 addition & 1 deletion packages/ods/vue/tests/e2e/ods-link.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('ods-link vue', () => {
await goToComponentPage(page, 'ods-link');
});

it('render the component correctly', async () => {
it('should navigate on click', async () => {
const elem = await page.$('ods-link');

await elem?.click();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Meta } from '@storybook/addon-docs';
import { Meta } from '@storybook/blocks';
import SpecificationsLink from '@ovhcloud/ods-components/src/components/link/documentation/specifications/specifications-link.mdx';

<Meta title="ODS Components/Actions/Link/Specifications" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Meta } from '@storybook/addon-docs';
import { Meta } from '@storybook/blocks';
import Usage from '@ovhcloud/ods-components/src/components/link/documentation/usage-guidelines/usage.mdx';

<Meta title="ODS Components/Actions/Link/Usage Guidelines" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Meta } from '@storybook/addon-docs';
import { Meta } from '@storybook/blocks';
import Migration from '@ovhcloud/ods-components/src/components/link/documentation/migration.from.17.x.mdx';

<Meta title="ODS Components/Actions/Link/Migration From 17.x" />
Expand Down

0 comments on commit 72db99a

Please sign in to comment.