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

Icon Font Update #284

Merged
merged 8 commits into from
Dec 4, 2014
Merged

Icon Font Update #284

merged 8 commits into from
Dec 4, 2014

Conversation

cshold
Copy link
Contributor

@cshold cshold commented Dec 2, 2014

Updated icon fonts to include. Docs to be updated after this PR is merged.

  • Hamburger
  • Plus
  • Minus
  • Grid view
  • List view

Also moved the inline collection view JS into shop.js.

This lets us delete the inline svg icons previously used for collection views, while adding a few more commonly used icons to the set.

Edit: also moved collections and blog sidebars below main content with grid--rev on mobile sizes. Should be its own PR, but minor enough I tacked it on here.

@mpiotrowicz @stevebosworth

@@ -104,6 +104,9 @@
- Documentation: http://shopify.com/timber#ajax-cart
{% endcomment %}
<a href="/cart" id="CartToggle">
<span class="icon-fallback-text">
Copy link
Contributor

Choose a reason for hiding this comment

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

there's no copy in this fallback text, is it needed? Fallback should be there if an icon isn't accompanied by any visible text or if the text isn't representative of what the icon shows. eg. if the icon wasn't there, would something still be actionable and would users know what it did? cc @stevebosworth

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a superfluous icon, really. There already is text beside it, so if it loads, great, if not, not a problem.

screen shot 2014-12-03 at 9 13 55 am

Copy link
Contributor

Choose a reason for hiding this comment

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

ah ok, with the comment below as well i see how this is organized. If there's no fallback text required, just wondering if it needs to be wrapped in that element at all? I can open a separate issue elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see what you mean. Yea, if there is no fallback the wrapper can be
removed. The default .icon:before is hidden unless icon fonts are
supported. Please open a new issue and I can revisit this.

On Wed, Dec 3, 2014 at 9:41 AM, Monika Piotrowicz notifications@github.com
wrote:

In layout/theme.liquid:

@@ -104,6 +104,9 @@
- Documentation: http://shopify.com/timber#ajax-cart
{% endcomment %}

  •        <span class="icon-fallback-text">
    

ah ok, with the comment below as well i see how this is organized. If
there's no fallback text required, just wondering if it needs to be wrapped
in that element at all? I can open a separate issue elsewhere


Reply to this email directly or view it on GitHub
https://github.com/Shopify/Timber/pull/284/files#r21234684.

Carson Shold
Front End Web Developer

<button type="button" title="Grid view" class="change-view{% unless template contains 'list' %} change-view--active{% endunless %}" data-view="grid">
<span class="icon-fallback-text">
<span class="icon icon-grid-view" aria-hidden="true"></span>
<span class="fallback-text">Grid View</span>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonasll Could I get a translation for "Grid View" and "List View" please.

@jonasll
Copy link
Contributor

jonasll commented Dec 3, 2014

fr

Grid View: Grille
List View: Liste

pt

Grid View: Grelha
List View: Lista

pt-BR

Grid View: Grelha
List View: Lista

de

Grid View: Gitteransicht
List View: Listenansicht

es

Grid View: Tabla
List View: Lista

cshold added a commit that referenced this pull request Dec 4, 2014
@cshold cshold merged commit d09b77c into master Dec 4, 2014
@cshold cshold deleted the font-update branch December 4, 2014 14:01
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.

3 participants