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

fix(listing): remove extra padding - INNO-916 #686

Merged
merged 7 commits into from
May 7, 2018
Merged

Conversation

emeryro
Copy link
Contributor

@emeryro emeryro commented May 2, 2018

PR description

Fix horizontal alignment

QA Checklist

In order to ensure a safe and quick review, please check that your PR follow those guidelines:

  • package.json is up-to-date and @ecl/[system]-base is part of the dependencies
  • I have checked the dependencies
  • I have given the fractal status “ready” to my component
  • I have declared @define mycomponent in the SCSS file
  • I have specified margin: 0; on the CSS component
  • I have provided tests
  • I follow the naming guidelines
  • the component supports composition
  • there are no hardcoded strings (all content come from the context)
  • I have filled the README.md file (at least a few lines)
  • My component is listed in the root README
  • My PR has the right label(s)

@HAL-Patch-INNO
Copy link
Contributor

HAL-Patch-INNO commented May 2, 2018

Deploy preview for europa-component-library ready!

Built with commit 29534d5

https://deploy-preview-686--europa-component-library.netlify.com

@kalinchernev kalinchernev self-assigned this May 7, 2018
Copy link
Contributor

@kalinchernev kalinchernev left a comment

Choose a reason for hiding this comment

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

My only comment would be to make the logic a bit more easy to understand, otherwise it works well :)

@@ -31,8 +31,12 @@
width: 50%;
}

.ecl-list-item__link {
.ecl-list-item:nth-child(2n + 0) .ecl-list-item__link {
Copy link
Contributor

Choose a reason for hiding this comment

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

To be honest, it's easy to understand what's going on here without reading aside. A comment or two what are these calculations meant to represent will be helpful. There's also the nth-of-type(odd) and nth-of-type(even) syntax which makes it a bit more obvious without the calculations.

@@ -57,10 +61,18 @@
width: 33%;
}

.ecl-list-item__link {
.ecl-list-item:nth-child(3n + 0) .ecl-list-item__link {
Copy link
Contributor

Choose a reason for hiding this comment

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

Some documentation here will also be useful :)

@kalinchernev kalinchernev removed their assignment May 7, 2018
@degliwe degliwe merged commit 976fc42 into master May 7, 2018
@degliwe degliwe deleted the fix/listing-INNO-916 branch May 7, 2018 14:07
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.

5 participants