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.

Sign up for GitHub

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

Jump to bottom

Add glyphs for new treatment page #60

Merged
merged 5 commits into from
Mar 4, 2019
Merged

Add glyphs for new treatment page #60

merged 5 commits into from
Mar 4, 2019

Conversation

Yihwan
Copy link
Contributor

@Yihwan Yihwan commented Mar 1, 2019 •
edited
Loading

Added glyphs for new Treatment Page.

screen shot 2019-03-01 at 3 15 03 pm

screen shot 2019-03-01 at 3 15 08 pm

screen shot 2019-03-01 at 3 14 57 pm

@Yihwan Yihwan requested review from snags88 and jcmustin March 1, 2019 18:13
@snags88 snags88 temporarily deployed to curology-radiance-pr-60 March 1, 2019 18:13 Inactive
@snags88
Copy link
Contributor

snags88 commented Mar 1, 2019

@Yihwan you don't have to worry about the size on the radiance storybook. By default, we size everything to 25px x 25px so if the glyph sizing will change based on their viewbox.

For the color, you need to remove some of the masks on the SVG definition

snags88
snags88 reviewed Mar 1, 2019
View reviewed changes
src/svgs/glyphs/consult-history-glyph.svg Outdated
@@ -0,0 +1,15 @@
<svg viewBox="0 0 48 48" fill="none" xmlns="http://www.w3.org/2000/svg">
<mask id="mask0" mask-type="alpha" maskUnits="userSpaceOnUse" x="15" y="16" width="20" height="20">
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove this

src/svgs/glyphs/consult-history-glyph.svg Outdated
<path fill-rule="evenodd" clip-rule="evenodd" d="M22.663 19.4663V17.7333H27.7223V19.4663H22.663ZM22.6204 15.9333C21.6248 15.9333 20.863 16.7571 20.863 17.7166V19.483C20.863 20.4425 21.6248 21.2663 22.6204 21.2663H27.7649C28.7605 21.2663 29.5223 20.4425 29.5223 19.483V17.7166C29.5223 16.7571 28.7605 15.9333 27.7649 15.9333H22.6204ZM32.1955 18.4903C31.8493 18.1337 31.2795 18.1252 30.9228 18.4714C30.5662 18.8177 30.5578 19.3874 30.904 19.7441C31.0599 19.9047 31.152 20.128 31.152 20.3663V32.7315C31.152 32.9698 31.0599 33.1931 30.904 33.3537C30.749 33.5133 30.5445 33.5979 30.3372 33.5979H20.0481C19.8408 33.5979 19.6363 33.5133 19.4813 33.3537C19.1351 32.997 18.5653 32.9886 18.2087 33.3348C17.852 33.681 17.8436 34.2508 18.1898 34.6075C18.678 35.1103 19.3459 35.3979 20.0481 35.3979H30.3372C31.0394 35.3979 31.7073 35.1103 32.1955 34.6075C32.6828 34.1055 32.952 33.4302 32.952 32.7315V20.3663C32.952 19.6676 32.6828 18.9923 32.1955 18.4903Z" fill="#332E54"/>
</g>
<rect x="15" y="16" width="14" height="15" fill="white"/>
<mask id="mask1" mask-type="alpha" maskUnits="userSpaceOnUse" x="12" y="13" width="20" height="20">
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove this

src/svgs/glyphs/consult-history-glyph.svg Outdated
@@ -0,0 +1,15 @@
<svg viewBox="0 0 48 48" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd consider renaming this to clipboards-glyph since this doesn't have to be strictly used for consultation history

@Yihwan Yihwan temporarily deployed to curology-radiance-pr-60 March 1, 2019 22:31 Inactive
@jcmustin
Copy link
Contributor

jcmustin commented Mar 1, 2019 •
edited
Loading

The color is grey because of the fill property on each path; it's commonly set to currentColor elsewhere in radiance. There's a white rectangle in there too that I think is plausibly extraneous?

Unless the white fill is necessary, I think this should be the minimal code necessary to render the ClipboardStackGlyph:

<svg viewBox="0 0 48 48" xmlns="http://www.w3.org/2000/svg">
    <path fill-rule="evenodd" d="M32.1955 18.4903C31.8493 18.1337 31.2795 18.1252 30.9228 18.4714C30.5662 18.8177 30.5578 19.3874 30.904 19.7441C31.0599 19.9047 31.152 20.128 31.152 20.3663V32.7315C31.152 32.9698 31.0599 33.1931 30.904 33.3537C30.749 33.5133 30.5445 33.5979 30.3372 33.5979H20.0481C19.8408 33.5979 19.6363 33.5133 19.4813 33.3537C19.1351 32.997 18.5653 32.9886 18.2087 33.3348C17.852 33.681 17.8436 34.2508 18.1898 34.6075C18.678 35.1103 19.3459 35.3979 20.0481 35.3979H30.3372C31.0394 35.3979 31.7073 35.1103 32.1955 34.6075C32.6828 34.1055 32.952 33.4302 32.952 32.7315V20.3663C32.952 19.6676 32.6828 18.9923 32.1955 18.4903Z" fill="currentColor"/>
    <path fill-rule="evenodd" d="M19.663 16.4663V14.7333H24.7224V15.5939C24.7224 15.5959 24.7224 15.5978 24.7224 15.5998C24.7224 15.6018 24.7224 15.6038 24.7224 15.6057V16.4663H19.663ZM26.5223 16.4998C26.5135 17.4521 25.7547 18.2663 24.765 18.2663H19.6205C18.6307 18.2663 17.872 17.4521 17.8631 16.4998H17.0482C16.8408 16.4998 16.6364 16.5844 16.4814 16.7441C16.3255 16.9047 16.2334 17.128 16.2334 17.3663V29.7315L16.2333 29.7315C16.2334 29.9698 16.3255 30.1931 16.4814 30.3537L16.4814 30.3537C16.5395 30.4136 16.6046 30.4629 16.6742 30.5011C16.7901 30.5649 16.9186 30.5979 17.0482 30.5979H27.3372C27.5446 30.5979 27.749 30.5133 27.904 30.3537C28.0359 30.2179 28.1221 30.0372 28.1456 29.8405C28.1481 29.8192 28.1499 29.7977 28.151 29.776C28.1517 29.7612 28.1521 29.7464 28.1521 29.7315V17.3663C28.1521 17.3096 28.1469 17.2537 28.1368 17.1994C28.1099 17.0538 28.0483 16.919 27.9595 16.8072C27.942 16.7852 27.9235 16.7642 27.9041 16.7441C27.749 16.5844 27.5446 16.4998 27.3372 16.4998H26.5223ZM26.5223 14.6998H27.3372C28.0395 14.6998 28.7074 14.9874 29.1956 15.4903C29.2078 15.5029 29.2198 15.5155 29.2317 15.5283C29.5771 15.8985 29.8071 16.3577 29.9024 16.849C29.9352 17.0184 29.9521 17.1916 29.9521 17.3663V29.7315C29.9521 29.8188 29.9479 29.9058 29.9395 29.9921C29.8814 30.5963 29.622 31.1683 29.1956 31.6075C28.7074 32.1104 28.0395 32.398 27.3372 32.398H17.0482C16.5865 32.398 16.1396 32.2736 15.7489 32.0455C15.6197 31.9701 15.4965 31.8832 15.3811 31.7858C15.3147 31.7298 15.2509 31.6703 15.1899 31.6075L15.1899 31.6075C14.9462 31.3565 14.7571 31.0622 14.6289 30.7433C14.5007 30.4243 14.4333 30.0808 14.4333 29.7315V17.3663C14.4333 16.6676 14.7026 15.9923 15.1899 15.4903C15.678 14.9874 16.3459 14.6998 17.0482 14.6998H17.8631C17.8719 13.7475 18.6307 12.9333 19.6205 12.9333H24.765C25.7548 12.9333 26.5135 13.7475 26.5223 14.6998Z" fill="currentColor"/>
</svg>

@Yihwan Yihwan temporarily deployed to curology-radiance-pr-60 March 1, 2019 23:14 Inactive
@Yihwan Yihwan requested a review from snags88 March 1, 2019 23:15
jcmustin
jcmustin approved these changes Mar 2, 2019
View reviewed changes
Copy link
Contributor

@jcmustin jcmustin left a comment

Choose a reason for hiding this comment

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

These look good to me! 👍

snags88
snags88 approved these changes Mar 3, 2019
View reviewed changes
Copy link
Contributor

@snags88 snags88 left a comment

Choose a reason for hiding this comment

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

I'd leave out the changelog on a PR since the PR might not be the only thing going on the version. You can just commit the changelog when you deploy a new version of the package on NPM

@Yihwan Yihwan requested a deployment to curology-radiance-pr-60 March 4, 2019 17:26 Abandoned
@Yihwan Yihwan merged commit f5f2446 into master Mar 4, 2019
@Yihwan Yihwan deleted the add-treatment-glyphs branch March 4, 2019 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcmustin jcmustin jcmustin approved these changes

@snags88 snags88 snags88 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Yihwan @snags88 @jcmustin