Skip to content
This repository has been archived by the owner on Aug 30, 2018. It is now read-only.

Icons and Modernizer Adjustments #85

Merged
merged 9 commits into from
Jun 4, 2014
Merged

Icons and Modernizer Adjustments #85

merged 9 commits into from
Jun 4, 2014

Conversation

cshold
Copy link
Contributor

@cshold cshold commented Jun 3, 2014

  • Moving Modernizer in to its own file and included separately
  • Standardized icon font approach to be css-centric
    • Text fallback when font-face not supported
  • Limit social links to Facebook, Twitter, Pinterest (more can be added easily)
  • Simplified payment icon generation

{% if type == "stripe" %}<li>S</li>{% endif %}
{% if type == "bitcoin" %}<li>B</li>{% endif %}
<li>
<span class="icon-fallback-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.

@tobi @mpiotrowicz Moved to a much simpler icon approach with text fallback when font-face isn't supported.

@cshold cshold mentioned this pull request Jun 3, 2014
<li>
<span class="icon-fallback-text">
<span class="icon icon-{{type}}" aria-hidden="true"></span>
<span class="fallback-text">{{type | replace: '_',' '}}</span>
Copy link
Member

Choose a reason for hiding this comment

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

we should just make type.name work instead of relying on magical replace. @markdunkley know someone who could do that?

Choose a reason for hiding this comment

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

@gauravmc thoughts? {{ type | name }}

Copy link

Choose a reason for hiding this comment

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

@tobi making {{ type.name }} work will mean making some drop instead of simple type string right?

It'll be simple enough to add a filter as @markdunkley's suggesting. We can add a humanize filter that replaces underscores with spaces. @cshold is such operation common?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't say common, though I'm also not very familiar with similar languages. humanize has a nice ring to it though.

Copy link
Member

Choose a reason for hiding this comment

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

I like humanize as a filter.

-- Tobi
CEO Shopify

(mobile)

On Jun 3, 2014, at 7:05 PM, Carson Shold notifications@github.com wrote:

In snippets/footer.liquid:

  •    {% if type == "interac" %}<li>I</li>{% endif %}
    
  •    {% if type == "visa" %}<li>V</li>{% endif %}
    
  •    {% if type == "master" %}<li>M</li>{% endif %}
    
  •    {% if type == "discover" %}<li>D</li>{% endif %}
    
  •    {% if type == "dk" %}<li>d</li>{% endif %}
    
  •    {% if type == "american_express" %}<li>A</li>{% endif %}
    
  •    {% if type == "google" %}<li>G</li>{% endif %}
    
  •    {% if type == "paypal" %}<li>P</li>{% endif %}
    
  •    {% if type == "jcb" %}<li>J</li>{% endif %}
    
  •    {% if type == "cirrus" %}<li>C</li>{% endif %}
    
  •    {% if type == "stripe" %}<li>S</li>{% endif %}
    
  •    {% if type == "bitcoin" %}<li>B</li>{% endif %}
    
  •    <li>
    
  •      <span class="icon-fallback-text">
    
  •        <span class="icon icon-{{type}}" aria-hidden="true"></span>
    
  •        <span class="fallback-text">{{type | replace: '_',' '}}</span>
    
    I wouldn't say common, though I'm also not very familiar with similar languages. humanize has a nice ring to it though.


Reply to this email directly or view it on GitHub.

cshold added a commit that referenced this pull request Jun 4, 2014
Icons and Modernizer Adjustments
@cshold cshold merged commit ee6b828 into master Jun 4, 2014
@cshold cshold deleted the payment-icons branch June 6, 2014 12:58
cimocimocimo pushed a commit to cimocimocimo/Theia-Shopify-Theme that referenced this pull request Nov 25, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants