Add button styles to add and delete #571
Conversation
|
@poofichu Styling thoughts for you:
|
Snugug
changed the title
Snugug
changed the title
|
@ineedsubstance thoughts on the placement/button style of
|
|
@poofichu - I like this version where the |
|
@ineedsubstance Ask and you shall receive:
|
|
@poofichu - Wow, nice! I like it. Makes it stand out a little more and puts more emphasis on the purpose of the button. As long as you are cool with it, let's roll with it. |
…6-addbutton especially if it merges an updated upstream into a topic branch. Update branch
| @@ -0,0 +1,4 @@ | |||
| <svg id="Layer_1" data-name="Layer 1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 9.5 9.5"> | |||
There was a problem hiding this comment.
Can we put this in src/images/icons please
There was a problem hiding this comment.
And add all of the icon classes to this, please
| background-image: url('/images/add.svg'); | ||
| background-position: setting-get('rhythm') 49%; | ||
| background-repeat: no-repeat; | ||
| background-size: 9px; |
There was a problem hiding this comment.
Background size in em instead of px please. Generally don't use px
| @include button(setting-get('primary accent color'), setting-get('primary accent color'), setting-get('primary color'), setting-get('primary accent color'), setting-get('primary color'), setting-get('primary color'), setting-get('primary color')); | ||
|
|
||
| background-image: url('/images/add.svg'); | ||
| background-position: setting-get('rhythm') 49%; |
There was a problem hiding this comment.
Why is this 49%? Why not 50%? If it's because it doesn't vertically or horizontally center properly, use calc to nudge it in to the right position
| &--add { | ||
| @include button(setting-get('primary accent color'), setting-get('primary accent color'), setting-get('primary color'), setting-get('primary accent color'), setting-get('primary color'), setting-get('primary color'), setting-get('primary color')); | ||
|
|
||
| background-image: url('/images/add.svg'); |
There was a problem hiding this comment.
We aren't going to be able to guarantee that this background image is always going to be there. This should be an inline SVG in the HTML
There was a problem hiding this comment.
If you have problems with that, extend https://github.com/punchcard-cms/punchcard/blob/master/lib/init/views.js like we did before
There was a problem hiding this comment.
@Snugug So if we're going to add the svg to the html I need to do so in this file, yah?
https://github.com/punchcard-cms/content-types/blob/master/lib/form/html.js#L179
Can you do nunjucks partials in js files?
|
@poofichu are changes forthcoming this day, the day of monday, the day after a holiday, changes shall come forth, so be it and so it is said? |
|
@scottnath going to work on this today! Problem we are having is how to inject the svg code for a |
|
|
||
| background-color: transparent; | ||
| border: { | ||
| left: 0 none; |
There was a problem hiding this comment.
If the width of all of these is 0 then they don't need their style set to none
| &--add { | ||
| @include button(setting-get('primary accent color'), setting-get('primary accent color'), setting-get('primary color'), setting-get('primary accent color'), setting-get('primary color'), setting-get('primary color'), setting-get('primary color')); | ||
|
|
||
| background-image: url('/images/icons/add.svg'); |
There was a problem hiding this comment.
This may be easier for us to read if image, position, repeat, and size were all nested under background like you're doing with border below
Closes #556