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

Footer #158

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Footer #158

wants to merge 4 commits into from

Conversation

misscs
Copy link
Contributor

@misscs misscs commented Jun 13, 2016

  • Prepare footer for international
  • Improve/simplify markup

commit f3387cb
Author: claudina sarahe <[email protected]>
Date:   Mon Jun 13 04:59:03 2016 -0400

    Make imports relative paths

    - Imports need to be defined relative to nightshade-core/src in order to avoid
    errors. This resolves errors when running storefront-static or nightshade

commit a1737c2
Author: claudina sarahe <[email protected]>
Date:   Mon Jun 13 04:57:50 2016 -0400

    Add file comments
- Remove hard-coded content
- Simplify html
@@ -7,53 +7,42 @@
{#
* Renders a standard or simplified footer
#}
{% macro footer(simple=false, class=null, id=null, phone_number='888–498–0003') %}
{% macro footer(data, simple=false, class=null, id=null) %}
<footer class="footer{% if simple %} footer--simple{% endif %}{% if class %} {{ class }}{% endif %}"{% if id %} id="{{ id }}"{% endif %}>
<div class="footer-top r-inner">
{{ caller() }}
</div>
<div class="footer-bottom">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's a better class to replace footer-bottom? I like the feel of the the site- classes to describe the content, but it now feels disconnected from its parent class.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a huge fan of class names that describe position, since the positions could theoretically be made incorrect in certain media queries via CSS. footer-bottom seems like navigation - could this be footer-nav?

@misscs
Copy link
Contributor Author

misscs commented Jun 13, 2016

@CasperSleep/frontier Left a couple comments looking for some input on class names. Thank you!

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

Successfully merging this pull request may close these issues.

3 participants