Skip to content

Commit

Permalink
BREAKING CHANGE(web-twig): Remove onClick prop #DS-686
Browse files Browse the repository at this point in the history
 ## Migration Guide

Instead of the `onClick` prop, use native `onclick` in the `Button`, `ButtonLink`,
`HeaderLink`, `HeaderDialogLink` and `Link` components.

- `<Button onClick="alert('alert');" …>` → `<Button onclick="alert('alert');" …>`
- `<ButtonLink onClick="alert('alert');" …>` → `<ButtonLink onclick="alert('alert');" …>`
- `<HeaderLink onClick="alert('alert');" …>` → `<HeaderLink onclick="alert('alert');" …>`
- `<HeaderDialogLink onClick="alert('alert');" …>`
→ `<HeaderDialogLink onclick="alert('alert');" …>`
- `<Link onClick="alert('alert');" …>` → `<Link onclick="alert('alert');" …>`

Please refer back to this guide or reach out to our team
if you encounter any issues during migration.
  • Loading branch information
crishpeen authored and literat committed Jul 21, 2023
1 parent ab5606a commit ab62f78
Show file tree
Hide file tree
Showing 17 changed files with 20 additions and 61 deletions.
8 changes: 0 additions & 8 deletions packages/web-twig/src/Resources/components/Button/Button.twig
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
{%- set _isDisabled = props.isDisabled | default(false) | boolprop -%}
{%- set _isLoading = props.isLoading | default(false) | boolprop -%}
{%- set _isSquare = props.isSquare | default(false) | boolprop -%}
{%- set _onClick = props.onClick | default(null) -%}
{%- set _type = props.type | default('button') -%}

{# Class names #}
Expand All @@ -19,25 +18,18 @@

{# Attributes #}
{%- set _disabledAttr = _isDisabled or _isLoading ? 'disabled' : null -%}
{%- set _onClickAttr = _onClick ? 'onclick=' ~ _onClick | escape('html_attr') : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
{%- set _classNames = [ _rootClassName, _rootColorClassName, _rootSizeClassName, _rootBlockClassName, _rootLoadingClassName, _rootSquareClassName, _styleProps.className ] -%}
{%- set _allowedAttributes = [ 'name' ] -%}

{# Deprecations #}
{% if _onClick %}
{% deprecated 'Button: The "onClick" property will be removed in the next major version. Use native "onclick" attribute instead.' %}
{% endif %}

<button
{{ mainProps(props, _allowedAttributes) }}
{{ styleProp(_styleProps) }}
{{ classProp(_classNames) }}
type="{{ _type }}"
{{ _disabledAttr }}
{{ _onClickAttr | raw }}
>
{%- block content %}{% endblock %}
{%- if _isLoading -%}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ Without lexer:
| `isLoading` | `bool` | `false` | no | If true, Button is in a loading state, disabled and the Spinner is visible |
| `isSquare` | `bool` | `false` | no | If true, Button is square, usually only with an Icon |
| `name` | `string` | `null` | no | For use a button as a form data reference |
| `onClick` | `string` | `null` | no | JS function to call on click |
| `size` | [Size dictionary][dictionary-size] | `medium` | no | Size variant |
| `type` | `string` | `button` | no | Type of the Button |

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
{% for color in colors %}
<div class="{% if color is same as('inverted') %}docs-Box {% endif %}my-600">
<div>
<Button color={color} size={size} onClick="alert('rimmer')">Button {{ color }}</Button>
<Button color={color} size={size} onclick="alert('Button clicked')">Button {{ color }}</Button>
<Button color={color} size={size}>
<Icon name="hamburger" UNSAFE_className="mr-400" />
Menu
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
{%- set _isDisabled = props.isDisabled | default(false) | boolprop -%}
{%- set _isLoading = props.isLoading | default(false) | boolprop -%}
{%- set _isSquare = props.isSquare | default(false) | boolprop -%}
{%- set _onClick = props.onClick | default(null) -%}

{# Class names #}
{%- set _rootClassName = _spiritClassPrefix ~ 'Button' -%}
Expand All @@ -18,9 +17,6 @@
{%- set _rootSizeClassName = _spiritClassPrefix ~ 'Button--' ~ _size -%}
{%- set _rootSquareClassName = _isSquare ? _spiritClassPrefix ~ 'Button--square' : null -%}

{# Attributes #}
{%- set _onClickAttr = _onClick ? 'onclick=' ~ _onClick | escape('html_attr') : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
{%- set _classNames = [ _rootClassName, _rootColorClassName, _rootBlockClassName, _rootDisabledClassName, _rootLoadingClassName, _rootSizeClassName, _rootSquareClassName, _styleProps.className ] -%}
Expand All @@ -29,17 +25,11 @@
'target',
] -%}

{# Deprecations #}
{% if _onClick %}
{% deprecated 'ButtonLink: The "onClick" property will be removed in the next major version. Use native "onclick" attribute instead.' %}
{% endif %}

<a
{{ mainProps(props, _allowedAttributes) }}
{{ styleProp(_styleProps) }}
{{ classProp(_classNames) }}
href="{{ _href }}"
{{ _onClickAttr | raw }}
>
{%- block content %}{% endblock %}
{%- if _isLoading -%}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ Without lexer:
| `isDisabled` | `bool` | `false` | no | If true, ButtonLink is disabled |
| `isLoading` | `bool` | `false` | no | If true, ButtonLink is in a loading state, disabled and the Spinner is visible |
| `isSquare` | `bool` | `false` | no | If true, ButtonLink is square, usually only with an Icon |
| `onClick` | `string` | `null` | no | JS function to call on click |
| `target` | `string` | `null` | no | Link target |
| `title` | `string` | `null` | no | Optional title to display on hover |

Expand Down
22 changes: 10 additions & 12 deletions packages/web-twig/src/Resources/components/Header/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,11 @@ There is no API for HeaderNavItem.

##### HeaderLink API

| Prop name | Type | Default | Required | Description |
| ----------- | --------- | ------- | -------- | ------------------------- |
| `href` | `string` || yes | Link URL |
| `isCurrent` | `boolean` | `false` | no | Mark link as current |
| `onClick` | `string` | `null` | no | Function to call on click |
| `target` | `string` | `null` | no | HTML `target` attribute |
| Prop name | Type | Default | Required | Description |
| ----------- | --------- | ------- | -------- | ----------------------- |
| `href` | `string` || yes | Link URL |
| `isCurrent` | `boolean` | `false` | no | Mark link as current |
| `target` | `string` | `null` | no | HTML `target` attribute |

##### HeaderButton API

Expand Down Expand Up @@ -391,12 +390,11 @@ There is no API for HeaderDialogNavItem.

##### HeaderDialogLink API

| Prop name | Type | Default | Required | Description |
| ----------- | --------- | ------- | -------- | ------------------------- |
| `href` | `string` || yes | Link URL |
| `isCurrent` | `boolean` | `false` | no | Mark link as current |
| `onClick` | `string` | `null` | no | Function to call on click |
| `target` | `string` | `null` | no | HTML `target` attribute |
| Prop name | Type | Default | Required | Description |
| ----------- | --------- | ------- | -------- | ----------------------- |
| `href` | `string` || yes | Link URL |
| `isCurrent` | `boolean` | `false` | no | Mark link as current |
| `target` | `string` | `null` | no | HTML `target` attribute |

##### HeaderDialogButton API

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
{%- set _class = props.class | default(null) -%}
{%- set _href = props.href -%}
{%- set _isCurrent = props.isCurrent | default(false) -%}
{%- set _onClick = props.onClick | default(null) -%}
{%- set _rootClass = props.rootClass -%}

{# Class names #}
Expand All @@ -12,25 +11,18 @@

{# Attributes #}
{%- set _ariaCurrentAttr = _isCurrent ? 'aria-current=page' : null -%}
{%- set _onClickAttr = _onClick ? 'onclick=' ~ _onClick | escape('html_attr') : null -%}

{# Miscellaneous #}
{%- set _classNames = [ _rootClassName, _rootCurrentClassName, _class ] -%}
{%- set _allowedAttributes = [
'target',
] -%}

{# Deprecations #}
{% if _onClick %}
{% deprecated 'Header: The "onClick" property will be removed in the next major version. Use native "onclick" attribute instead.' %}
{% endif %}

<a
{{ mainProps(props, _allowedAttributes) }}
{{ classProp(_classNames) }}
href="{{ _href }}"
{{ _ariaCurrentAttr }}
{{ _onClickAttr | raw }}
>
{% block content %}{% endblock %}
</a>
10 changes: 0 additions & 10 deletions packages/web-twig/src/Resources/components/Link/Link.twig
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,12 @@
{%- set _href = props.href -%}
{%- set _isDisabled = props.isDisabled | default(false) | boolprop -%}
{%- set _isUnderlined = props.isUnderlined | default(false) | boolprop -%}
{%- set _onClick = props.onClick | default(null) -%}

{# Class names #}
{%- set _colorClassName = _spiritClassPrefix ~ 'link-' ~ _color -%}
{%- set _rootDisabledClassName = _isDisabled ? _spiritClassPrefix ~ 'link-disabled' : null -%}
{%- set _rootUnderlinedClassName = _isUnderlined ? _spiritClassPrefix ~ 'link-underlined' : null -%}

{# Attributes #}
{%- set _onClickAttr = _onClick ? 'onclick=' ~ _onClick | escape('html_attr') : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
{%- set _classNames = [ _colorClassName, _rootDisabledClassName, _rootUnderlinedClassName, _styleProps.className ] -%}
Expand All @@ -22,17 +18,11 @@
'target',
] -%}

{# Deprecations #}
{% if _onClick %}
{% deprecated 'Link: The "onClick" property will be removed in the next major version. Use native "onclick" attribute instead.' %}
{% endif %}

<a
{{ mainProps(props, _allowedAttributes) }}
href="{{ _href }}"
{{ styleProp(_styleProps) }}
{{ classProp(_classNames) }}
{{ _onClickAttr | raw }}
>
{%- block content %}{% endblock %}
</a>
1 change: 0 additions & 1 deletion packages/web-twig/src/Resources/components/Link/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ Without lexer:
| `href` | `string` || yes | Link URL |
| `isDisabled` | `bool` | `false` | no | If true, Link is disabled |
| `isUnderlined` | `bool` | `false` | no | If true, Link is underlined |
| `onClick` | `string` | `null` | no | JS function to call on click |
| `target` | `string` | `null` | no | Link target |
| `title` | `string` | `null` | no | Optional title to display on hover |

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
</title>
</head>
<body>
<button id="testId" name="testName" data-main="main" aria-label="label" class="Button Button--primary Button--medium" type="button" onclick="alert('&quot; test')">button</button> <button id="testId" name="testName" data-main="main" aria-label="label" onclick="alert('&quot; test')" class="Button Button--primary Button--medium" type="button">button</button> <button id="testId" name="testName" data-main="main" aria-label="label" class="Button Button--primary Button--medium" type="button" onclick="document.body.append('&lt;span&gt;hello there&lt;/span&gt;'); return false;">button</button> <button id="testId" name="testName" data-main="main" aria-label="label" onclick="document.body.append('&lt;span&gt;hello there&lt;/span&gt;'); return false;" class="Button Button--primary Button--medium" type="button">button</button>
<button id="testId" name="testName" data-main="main" aria-label="label" onclick="alert('&quot; test')" class="Button Button--primary Button--medium" type="button">button</button> <button id="testId" name="testName" data-main="main" aria-label="label" onclick="alert('&quot; test')" class="Button Button--primary Button--medium" type="button">button</button> <button id="testId" name="testName" data-main="main" aria-label="label" onclick="document.body.append('&lt;span&gt;hello there&lt;/span&gt;'); return false;" class="Button Button--primary Button--medium" type="button">button</button> <button id="testId" name="testName" data-main="main" aria-label="label" onclick="document.body.append('&lt;span&gt;hello there&lt;/span&gt;'); return false;" class="Button Button--primary Button--medium" type="button">button</button>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
<a class="HeaderDialogLink" href="/job-offers">Job offers</a> <!-- Render as current page -->
<a class="HeaderDialogLink HeaderDialogLink--current" href="/job-offers" aria-current="page">Job offers</a>
<!-- Render with all props -->
<a target="_blank" class="HeaderDialogLink HeaderDialogLink--current" href="https://spirit.design" aria-current="page" onclick="console.log('Hello!')">Job offers</a>
<a onclick="console.log('Hello!')" target="_blank" class="HeaderDialogLink HeaderDialogLink--current" href="https://spirit.design" aria-current="page">Job offers</a>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
<a class="HeaderLink" href="/job-offers">Job offers</a> <!-- Render as current page -->
<a class="HeaderLink HeaderLink--current" href="/job-offers" aria-current="page">Job offers</a>
<!-- Render with all props -->
<a target="_blank" class="HeaderLink HeaderLink--current" href="https://spirit.design" aria-current="page" onclick="console.log('Hello!')">Job offers</a>
<a onclick="console.log('Hello!')" target="_blank" class="HeaderLink HeaderLink--current" href="https://spirit.design" aria-current="page">Job offers</a>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
<a href="" class="link-primary">Example link</a> <!-- Render as an underlined secondary link -->
<a href="/" class="link-secondary link-underlined">Example link</a> <!-- Render as disabled -->
<a href="/" class="link-primary link-disabled">Example link</a> <!-- Render with all props -->
<a target="_blank" title="test title" href="https://spirit.design" class="link-inverted link-disabled link-underlined my-custom-class" onclick="console.log('Hello!');">Example link</a>
<a onclick="console.log('Hello!');" target="_blank" title="test title" href="https://spirit.design" class="link-inverted link-disabled link-underlined my-custom-class">Example link</a>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{% set onClickCallback = "alert('\" test')" %}

<Button id="testId" name="testName" data-main="main" aria-label="label" not-valid-prop="unexist" color="primary" onClick={onClickCallback}>button</Button>
<Button id="testId" name="testName" data-main="main" aria-label="label" not-valid-prop="unexist" color="primary" onclick={onClickCallback}>button</Button>
<Button id="testId" name="testName" data-main="main" aria-label="label" not-valid-prop="unexist" color="primary" onclick={onClickCallback}>button</Button>

{% set onClickAppend = "document.body.append('<span>hello there</span>'); return false;" %}

<Button id="testId" name="testName" data-main="main" aria-label="label" not-valid-prop="unexist" color="primary" onClick={onClickAppend}>button</Button>
<Button id="testId" name="testName" data-main="main" aria-label="label" not-valid-prop="unexist" color="primary" onclick={onClickAppend}>button</Button>
<Button id="testId" name="testName" data-main="main" aria-label="label" not-valid-prop="unexist" color="primary" onclick={onClickAppend}>button</Button>
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<HeaderDialogLink
href="https://spirit.design"
isCurrent
onClick="console.log('Hello!')"
onclick="console.log('Hello!')"
target="_blank"
>
Job offers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<HeaderLink
href="https://spirit.design"
isCurrent
onClick="console.log('Hello!')"
onclick="console.log('Hello!')"
target="_blank"
>
Job offers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
href="https://spirit.design"
isDisabled
isUnderlined
onClick="console.log('Hello!');"
onclick="console.log('Hello!');"
target="_blank"
title="test title"
>
Expand Down

0 comments on commit ab62f78

Please sign in to comment.