Polish before publishing #3
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pehota
force-pushed
the
chore/polish-before-publishing
branch
from
9f8d2d2 to
162ee7d
Compare
Svg nodes don't have props and when trying to set e.g. class using Html.Attributes.class on an svg, a run time error occurs. elm/svg#3
pehota
force-pushed
the
chore/polish-before-publishing
branch
2 times, most recently
from
570589c to
c019cdf
Compare
pehota
force-pushed
the
chore/polish-before-publishing
branch
from
c019cdf to
2f96f72
Compare
ChristophP
reviewed
Apr 3, 2020
README.md
Outdated
| module MyModule exposing (customHtml) | ||
|
|
||
| import Html exposing (Html) | ||
| import Svg.Attribute exposing (class) |
pehota
force-pushed
the
chore/polish-before-publishing
branch
from
abbdd8f to
f806239
Compare
ChristophP
reviewed
Apr 3, 2020
README.md
Outdated
| ## Icons Color | ||
| For convenience sake by default the icons will take the current color of their parent element. | ||
| If this is not the desired behaviour it can be changed using the CSS `fill` propery and setting it to the desired value by either using a class or an inline style. | ||
|
|
There was a problem hiding this comment.
I would add some things
For convenience sake by default the icons will take the current color of their parent element. So the preferred way to set colors would be to set something like
color: greenin CSS.
If this is not the desired behaviour, a different fill mode can be set via the fill attribute, or CSS fill property.
ChristophP
reviewed
Apr 3, 2020
build/index.js
Outdated
| .replace(new RegExp("Html.Attribute", "g"), "Svg.Attribute") | ||
| .replace( | ||
| /\[(viewBox\s+"[\s0-9]+")\]\s+\+\+/, | ||
| `$1 :: Svg.Attributes.fill "currentColor" :: ` |
There was a problem hiding this comment.
I think fill is enough because you are importing Svg.Attributes exposing (..) in the module
pehota
force-pushed
the
chore/polish-before-publishing
branch
from
f74ea63 to
279c071
Compare
ChristophP
reviewed
Apr 3, 2020
build/index.js
Outdated
|
|
||
|
|
||
| ## Customizing Icons Size | ||
| The icons \`viewBox\` is set to "0 0 20 20". To resize the icons set either \`widht\` or \`height\` CSS property using either a class or an inline style. The icons' aspect ratio will be kept, so there is no need of specifying both properties. |
build/index.js
Outdated
| # Icons | ||
| ## Customizing Icons Color | ||
| For convenience sake by default the icons will take the current color of their parent element. | ||
| If this is not the desired behaviour it can be changed using the CSS \`fill\` propery and setting it to the desired value by either using a class or an inline style. |
There was a problem hiding this comment.
I would add some things
For convenience sake by default the icons will take the current text color of their parent element. So the preferred way to set colors would be to set something like
color: greenin CSS.
If this is not the desired behaviour, a different fill mode can be set via the fill attribute, or CSS fill property.
pehota
force-pushed
the
chore/polish-before-publishing
branch
from
279c071 to
1536866
Compare
pehota
force-pushed
the
chore/polish-before-publishing
branch
from
1536866 to
a883256
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use
Svg.Attributeinstead ofHtml.Attributeto avoid a possible run time error.Update readme to include an attribution to Zondicons author and add note regarding using svg attributes.
Change package license to
BSD-3-Clause.