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(links): change link with icon display - INNO-1829 #1320

Merged
merged 4 commits into from Oct 4, 2019

Conversation

@yhuard
Copy link
Contributor

commented Oct 3, 2019

Please double check there's no side effects

@yhuard yhuard requested a review from ec-europa/inno as a code owner Oct 3, 2019
@HAL-Patch-INNO

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2019

Deploy preview for europa-component-library ready!

Built with commit 502c163

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

Copy link
Contributor

left a comment

Display of links on several lines is indeed better:
before
image

after
image

However, this has some side effect in several locations. Some of them are minor but still noticable (file download, footer, language list)
image

But in some cases it is breaking (before on left):
social media follow
image
social media share
image
event content types
image

So we could fix this at the component/template level, but there may be other broken cases on sites using ECL.

@emeryro emeryro removed their assignment Oct 4, 2019
@yhuard yhuard self-assigned this Oct 4, 2019
@yhuard yhuard added the process: WIP label Oct 4, 2019
yhuard added 3 commits Oct 4, 2019
@yhuard yhuard removed their assignment Oct 4, 2019
@emeryro emeryro self-assigned this Oct 4, 2019
@emeryro
emeryro approved these changes Oct 4, 2019
@planctus planctus self-assigned this Oct 4, 2019
@planctus

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2019

I have to say that in many of the components, the alignment of the icon is still questionable, i'd say that in some of the cases playing with that single pixel movement of the element doesn't really change the feeling that something is not in the right place, but i believe that in most of these situation the problem comes from the "wrong" proportions between text and icon (typically it happens with arrows, see breadcrumb or pagination).
Anyway, i tried to check all the components including a link implementation and i couldn't spot anything clearlt broken by this implementation.. ;)

@planctus planctus merged commit 2a7a565 into v2-dev Oct 4, 2019
3 checks passed
3 checks passed
Semantic Pull Request ready to be squashed
Details
continuous-integration/drone/push the build was successful
Details
deploy/netlify Deploy preview ready!
Details
@planctus planctus deleted the fix/icon-links-INNO-1829 branch Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.