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

Introducing footer #108

Merged
merged 36 commits into from Jan 26, 2017
Merged

Introducing footer #108

merged 36 commits into from Jan 26, 2017

Conversation

FKSI
Copy link
Contributor

@FKSI FKSI commented Jan 10, 2017

#38

@FKSI FKSI requested a review from davidknezic January 10, 2017 09:54
@davidknezic
Copy link
Contributor

Can be accessed here:
https://design.axa.com/feature-footer/components/footer/

Copy link
Contributor

@davidknezic davidknezic left a comment

Choose a reason for hiding this comment

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

Nice! I think we need to spend some time on the documentation and also simplify the markup a little bit more. I'll be able to join you finalizing the footer tomorrow.

<a class="footer-link " href="#" data-footer="${this.$element.selector}">
<svg class="footer-back-icon" viewBox="0 0 24 24">
<path fill-rule="evenodd" clip-rule="evenodd" d="M5.39 1.478L6.86-.025l10.3 10.522.004-.004 1.467 1.497-.003.005.004.004-1.472 1.503-.004-.005L6.9 23.975 5.434 22.48 15.688 12" data-reactid="37"></path>
</svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer if we rendered this markup in our templates already and then just switched classes (like .in, ``.is-visible) to hide or show.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be great!

import $ from 'jquery'
import registerPlugin from './register-plugin'

class Footer {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's try to make this obsolete. We have such menu components in several places (ex. in the main menu, in the mobile burger menu) and could write a menu plugin that works in all different scenarios. I'll have a look if I can find some already existing code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be great. I was inspired by the modal plugin :)

$footer-bottom-border: $color-indigo;
$footer-sides-offset: 50px;

.clear-gutter {
Copy link
Contributor

Choose a reason for hiding this comment

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

Watch out, this would be a utility and would need to go into the utilities folder. However, I'd add the margin reset to the correspnding classes directly, so the developer doesn't have to take care about the implementation specifics.


.footer-bottom-social-light {
@media (min-width: $screen-md-min) {
height: 50px
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 a semicolon missing.

{{#code-snippet "html"}}{{#get snippets "/components/footer/snippets/footer-light-lang-social"}}{{{.}}}{{/get}}{{/code-snippet}}

{{/inline}}
{{/page}}
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great to have a similar documentation approach on the footer page as we have on the header page.

  • Tell shortly what the footer is supposed to do and why it's important
  • Show a full blown example
  • List all the possible elements that can go into the footer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree, I was in the hurry to finish for the demo. But definitely need more comments.

@FKSI FKSI merged commit 0b507f0 into master Jan 26, 2017
@FKSI FKSI deleted the feature/footer branch January 26, 2017 10:04
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.

None yet

3 participants