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
feat(list): new checklist icons #1649
Conversation
affects: @fremtind/jkl-list slight change to list padding, now defaults to 4px left padding, instead of just setting it while inside a jkl-body, this could result in some regression in apps relying on this, however it should not break anything, just some pixel pushing might be nessesary to maintain perfection
packages/list/list.scss
Outdated
$jkl-marker-cross: url("data:image/svg+xml,%3Csvg class='jkl-icon jkl-icon--inherit' viewBox='0 0 20 21' fill='none' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath fill-rule='evenodd' clip-rule='evenodd' d='M10.6897 12.2047L16.7976 5.77481L16.0836 5.07422L10.0003 11.4783L3.92319 5.07428L3.20958 5.77524L9.31054 12.2044L2.5 19.374L3.21395 20.0746L9.99992 12.9308L16.7857 20.0816L17.4993 19.3807L10.6897 12.2047Z' fill='black' %3E%3C/path%3E%3C/svg%3E"); | ||
$jkl-marker-check: url("data:image/svg+xml,%3Csvg class='jkl-icon jkl-icon--inherit' viewBox='0 0 20 21' fill='none' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='M1 15.2168L5.24264 19.4594L19.3848 5.3173' stroke='black'%3E%3C/path%3E%3C/svg%3E"); | ||
|
||
$jkl-marker-cross: url("data:image/svg+xml,%3Csvg width='19' height='19' fill='none' xmlns='http://www.w3.org/2000/svg'%3E%3Ccircle cx='9.5' cy='9.5' r='8.75' stroke='%23AA1F23' stroke-width='1.5'/%3E%3Cpath fill='%23AA1F23' d='M5.54 12.68l7.14-7.1.84.85-7.13 7.09z'/%3E%3Cpath fill='%23AA1F23' d='M12.67 13.47l-7.13-7.1.85-.83 7.13 7.1z'/%3E%3C/svg%3E%0A");; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Er litt suboptimalt å hardkode inn fargene her. Prøvde med en scss funksjon til å encode svgen, men parcel likte det ikke i det hele tatt. Tar gjerne imot forslag for å kunne hente fargene inn hit dynamisk
padding-left: rem(4px); | ||
} | ||
&__item { | ||
padding-left: rem(4px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Denne kan nok gjøre litt endring for de som ikke pakker inn med jkl-body. Men valgte å ikke gjøre det breaking da det er snakk om 4px.
top: 0.25em; | ||
left: rem(-23px); | ||
top: 15%; | ||
left: clamp(-1em, -1rem ,rem(-23px)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, IE skjønner ikke dette, men IE da.. com'on
affects: @fremtind/jkl-list
Også, dette faller sammen når man prøver å sette lista som title-small feks, men da er du såpass langt ute på tur, at jeg har ikke gjort så stor innsats for å løse det. Men er løstbart ved å legge noen pixler padding på toppen av hver li, så tenker det er greit med mindre noen har en genial css funksjon for å gjette på hvor mye top skal være. |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit c89769d:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
big yay! 👍
affects: @fremtind/jkl-list
📥 Proposed changes
slight change to list padding, now defaults to 4px left padding, instead of just setting it while
inside a jkl-body, this could result in some regression in apps relying on this, however it should
not break anything, just some pixel pushing might be necessary to maintain perfection
☑️ Submission checklist
yarn build
works locally with my changes💬 Further comments