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.

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

New start button icon #1329

Closed
wants to merge 15 commits into from
Closed
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
WIP: Change to API to ask if button is start button
aliuk2012 committed May 9, 2019

Verified

This commit was signed with the committer’s verified signature. The key has expired.
danielleadams Danielle Adams
commit 97203ef311a8f6d54dc8581350ea9883ba01d273
2 changes: 1 addition & 1 deletion src/components/button/button.yaml
Original file line number Diff line number Diff line change
@@ -65,7 +65,7 @@ examples:
data:
text: Start now link button
href: '/'
classes: 'govuk-button--start'
isStartButton: true
- name: input
data:
element: input
24 changes: 22 additions & 2 deletions src/components/button/template.njk
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
{# Determine type of element to use, if not explicitly set -#}
{# Set classes for this component #}
{% set classNames = "govuk-button" %}


{% if params.classes %}
{% set classNames = classNames + " " + params.classes %}
{% endif %}
{% if params.disabled %}
{% set classNames = classNames + " govuk-button--disabled" %}
{% endif %}


{# Determine type of element to use, if not explicitly set -#}
{% if params.element %}
{% set element = params.element | lower %}
{% else %}
@@ -10,9 +21,14 @@
{% endif %}
{% endif %}

{% if params.isStartButton %}
{% set iconMarkUp = '<svg data-name="Layer 1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20" height="19px" width="30px"><path fill="currentColor" d="M7.5 0H0l10 10L0 20h7.5l10-10-10-10z"/></svg>' %}
Copy link
Contributor

@NickColley NickColley May 13, 2019

Choose a reason for hiding this comment

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

I think we can improve the inline SVG by doing the following:

  • remove 'data-name' attribute as it's not needed.
  • add .govuk-button__icon class instead of styling the 'svg' element, this'll make sure if anyone else wants to put svg icons into this button we won't style theirs unintentionally.
  • change height and width to be relative units (em), this'll make the icon scale alongside the text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First two points have been done.

{% set classNames = classNames + " govuk-button--start" %}
{% endif %}

{#- Define common attributes that we can use across all element types #}

{%- set commonAttributes %} class="govuk-button{% if params.classes %} {{ params.classes }}{% endif %}{% if params.disabled %} govuk-button--disabled{% endif %}"{% for attribute, value in params.attributes %} {{attribute}}="{{value}}"{% endfor %}{% endset %}
{%- set commonAttributes %} class="{{ classNames }}" {% for attribute, value in params.attributes %} {{attribute}}="{{value}}"{% endfor %}{% endset %}

{#- Define common attributes we can use for both button and input types #}

@@ -22,12 +38,16 @@

{%- if element == 'a' %}
<a href="{{ params.href if params.href else '#' }}" role="button" draggable="false" {{- commonAttributes | safe }}>
<span style="display:inline-block">
{{ params.html | safe if params.html else params.text }}
{{ iconMarkUp | safe }}
</span>
</a>

{%- elseif element == 'button' %}
<button {%- if params.value %} value="{{ params.value }}"{% endif %} {{- buttonAttributes | safe }} {{- commonAttributes | safe }}>
{{ params.html | safe if params.html else params.text }}
{{ iconMarkUp | safe }}
</button>

{%- elseif element == 'input' %}