Skip to content

Format SVGs #13

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

Merged
merged 1 commit into from
Aug 31, 2018
Merged

Format SVGs #13

merged 1 commit into from
Aug 31, 2018

Conversation

pauldraper
Copy link
Contributor

SVGs were not being formatted.

Copy link
Owner

@stringham stringham left a comment

Choose a reason for hiding this comment

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

Skipping formatting svg elements was intentional since svg elements usually have several attributes and seeing each of them on a separate line doesn't help much.

I think that:

<svg *ngIf="!small" xmlns="http://www.w3.org/2000/svg" width="111" height="111" viewBox="0 0 111 111">
    <g fill="none" fill-rule="evenodd" transform="scale(1.819672131)">
        <polygon class="primary" fill="#29AAE1" points="38 0 0 38 0 61 4 61 4 57 57.002 3.996 61 4 60.998 0"/>
        <polygon fill="#000000" fill-opacity=".35" points="4 61 4 57 0 61"/>
        <polygon fill="#000000" fill-opacity=".35" points="57 4.001 61 4.001 61 0"/>
    </g>
</svg>

Looks much better than:

<svg
    *ngIf="!small"
    xmlns="http://www.w3.org/2000/svg"
    width="111"
    height="111"
    viewBox="0 0 111 111"
>
    <g
        fill="none"
        fill-rule="evenodd"
        transform="scale(1.819672131)"
    >
        <polygon
            class="primary"
            fill="#29AAE1"
            points="38 0 0 38 0 61 4 61 4 57 57.002 3.996 61 4 60.998 0"
        ></polygon>
        <polygon
            fill="#000000"
            fill-opacity=".35"
            points="4 61 4 57 0 61"
        ></polygon>
        <polygon
            fill="#000000"
            fill-opacity=".35"
            points="57 4.001 61 4.001 61 0"
        ></polygon>
    </g>
</svg>

@pauldraper
Copy link
Contributor Author

pauldraper commented Aug 31, 2018

It really all depends though, right? Which HTML fragment is clearer?

<form class="myForm" method="get" action="/html/codes/html_form_handler.cfm">
  <p>
    <label>Name
      <input type="text" name="customer_name" required>
    </label> 
  </p>

  <p>
    <label>Phone 
      <input type="tel" name="phone_number">
    </label>
  </p>

  <p>
    <label>Email 
      <input type="email" name="email_address">
    </label>
  </p>

  <fieldset>
    <legend>Which taxi do you require?</legend>
    <p><label> <input type="radio" name="taxi" required value="car"> Car </label></p>
    <p><label> <input type="radio" name="taxi" required value="van"> Van </label></p>
    <p><label> <input type="radio" name="taxi" required value="tuktuk"> Tuk Tuk </label></p>
  </fieldset>

  <fieldset>
    <legend>Extras</legend>
    <p><label> <input type="checkbox" name="extras" value="baby"> Baby Seat </label></p>
    <p><label> <input type="checkbox" name="extras" value="wheelchair"> Wheelchair Access </label></p>
    <p><label> <input type="checkbox" name="extras" value="tip"> Stock Tip </label></p>
  </fieldset>
</form>

or

<form
    class="myForm"
    method="get"
    action="/html/codes/html_form_handler.cfm"
>
    <p>
        <label>
            Name
            <input
                type="text"
                name="customer_name"
                required
            >
        </label>
    </p>

    <p>
        <label>
            Phone
            <input
                type="tel"
                name="phone_number"
            >
        </label>
    </p>

    <p>
        <label>
            Email
            <input
                type="email"
                name="email_address"
            >
        </label>
    </p>

    <fieldset>
        <legend>Which taxi do you require?</legend>
        <p>
            <label>
                <input
                    type="radio"
                    name="taxi"
                    required
                    value="car"
                >
                Car
            </label>
        </p>
        <p>
            <label>
                <input
                    type="radio"
                    name="taxi"
                    required
                    value="van"
                >
                Van
            </label>
        </p>
        <p>
            <label>
                <input
                    type="radio"
                    name="taxi"
                    required
                    value="tuktuk"
                >
                Tuk Tuk
            </label>
        </p>
    </fieldset>

    <fieldset>
        <legend>Extras</legend>
        <p>
            <label>
                <input
                    type="checkbox"
                    name="extras"
                    value="baby"
                >
                Baby Seat
            </label>
        </p>
        <p>
            <label>
                <input
                    type="checkbox"
                    name="extras"
                    value="wheelchair"
                >
                Wheelchair Access
            </label>
        </p>
        <p>
            <label>
                <input
                    type="checkbox"
                    name="extras"
                    value="tip"
                >
                Stock Tip
            </label>
        </p>
    </fieldset>
</form>

Or which SVG?

<svg class="fill crisp" xmlns="http://www.w3.org/2000/svg">
  <g class="axis">
    <rect class="axis-line" [attr.x]="plot.left" [attr.y]="svg.h - plot.bottom" height="1" [attr.width]="svg.w - plot.left - plot.right"></rect>
    <rect *ngFor="let tick of xTicks" class="axis-tick" x="0" y="0" height="10" width="1" attr.transform="translate({{plot.left + (svg.w - plot.left - plot.right) * tick.x}} {{svg.h - plot.bottom}})"></rect>
  </g>
  <g class="axis">
    <rect class="axis-line" x="70" y="10" [attr.height]="svg.h - 40" width="1"></rect>
    <rect *ngFor="let tick of yTicks" class="axis-tick" x="0" y="0" height="1" width="10" attr.transform="translate({{plot.left - 10}} {{svg.h - plot.bottom - (svg.h - plot.bottom - plot.top) * tick}})"></rect>
  </g>
</svg>
<svg
    class="fill crisp"
    xmlns="http://www.w3.org/2000/svg"
>
    <g class="axis">
        <rect
            class="axis-line"
            [attr.x]="plot.left"
            [attr.y]="svg.h - plot.bottom"
            height="1"
            [attr.width]="svg.w - plot.left - plot.right"
        ></rect>
        <rect
            *ngFor="let tick of xTicks"
            class="axis-tick"
            x="0"
            y="0"
            height="10"
            width="1"
            attr.transform="translate({{plot.left + (svg.w - plot.left - plot.right) * tick.x}} {{svg.h - plot.bottom}})"
        ></rect>
    </g>
    <g class="axis">
        <rect
            class="axis-line"
            x="70"
            y="10"
            [attr.height]="svg.h - 40"
            width="1"
        ></rect>
        <rect
            *ngFor="let tick of yTicks"
            class="axis-tick"
            x="0"
            y="0"
            height="1"
            width="10"
            attr.transform="translate({{plot.left - 10}} {{svg.h - plot.bottom - (svg.h - plot.bottom - plot.top) * tick}})"
        ></rect>
    </g>
</svg>

Conventionally, formatters have a maximum line length and then add lines breaks only when that is exceeded.

angular-template-formatter has a simpler algorithm of a line break per attribute. Since Angular templates tend to have longish attributes, this works out pretty well. But if that assumption doesn't hold true, then the result looks comparatively poor, whether HTML or SVG.

@stringham
Copy link
Owner

I guess every time I've used an inline svg it has been used like a static image. If you are using angular features to make it dynamic then you benefit from the formatting.

@stringham stringham merged commit c727270 into stringham:master Aug 31, 2018
@David-Klemenc
Copy link

You should keep the svg-prefix for the next reason:

Namespacing

SVG Elements
Usually when you use an SVG element in an Angular template, you will need to prefix its name with the svg: namespace prefix, as is done here:

example:

<svg viewBox="0 0 250 250">
  <svg:polygon points="125,30 125,30 125,30 31.9,63.2 46.1,186.3 125,230 125,230 125,230 203.9,186.3 218.1,63.2" />
  <svg:polygon points="125,30 125,52.2 125,52.1 125,153.4 125,153.4 125,230 125,230 203.9,186.3 218.1,63.2 125,30" />
  <svg:path d="M125,52.1L66.8,182.6h0h21.7h0l11.7-29.2h49.4l11.7,29.2h0h21.7h0L125,52.1L125,52.1L125,52.1L125,52.1
      L125,52.1z M142,135.4H108l17-40.9L142,135.4z"/>
</svg>

This will let Angular know that it's supposed to create SVG elements instead of HTML ones.

You may not always need this prefix. In particular, when the root tag is in the same component as all of the SVG content, Angular is able to infer the correct namespace automatically. But as soon as you break that content up into multiple components, this inference will no longer be possible.

For this reason, I recommend always using the namespace prefix even in cases where it's not strictly necessary. Plus, it communicates to anyone reading the template code that SVG is being used.

link to original article

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants