-
Notifications
You must be signed in to change notification settings - Fork 86
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
[WNMGDS-2861] Adds PrintIcon component #3196
Conversation
fillRule="evenodd" | ||
clipRule="evenodd" | ||
d="M28 4.82812V12C30.2094 12 32 13.7906 32 16V23C32 23.5525 31.5525 24 31 24H28V30C28 31.1044 27.1044 32 26 32H6C4.89563 32 4 31.1044 4 30V24H1C0.4475 24 0 23.5525 0 23V16C0 13.7906 1.79063 12 4 12V2C4 0.895625 4.89563 0 6 0H23.1712C23.7019 0 24.2106 0.210625 24.5856 0.585625L27.4144 3.41375C27.7894 3.78937 28 4.2975 28 4.82812ZM8 28H24V22H8V28ZM24 14H8V4H20V7C20 7.5525 20.4475 8 21 8H24V14ZM25.5 17C25.5 17.8281 26.1719 18.5 27 18.5C27.8281 18.5 28.5 17.8281 28.5 17C28.5 16.1712 27.8281 15.5 27 15.5C26.1719 15.5 25.5 16.1712 25.5 17Z" | ||
fill="#262626" |
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.
This fill could probably be removed - including it would prevent the icon from filling into a different color.
SvgIcon.scss includes a rule setting fill: currentColor
that allows icons to fill to whatever color its parent defines. Setting this locally would prevent this icon from being as flexible as it could be.
{/* eslint-disable-next-line react/no-danger -- Story with known text */} | ||
<td dangerouslySetInnerHTML={{ __html: notes }} /> | ||
export const AvailableIcons = () => { | ||
return ( |
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.
Did your linter change this? I don't believe adding the curly braces + return was necessary since arrow funcs have implicit returns
@@ -57,6 +57,7 @@ | |||
"lock": "Candado", | |||
"menu": "Icono de menú", | |||
"next": "Siguiente", | |||
"print": "Imprimir", |
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.
Not sure if you're bilingual. If not, you will probably want to confirm that this word is the correct one for this verb in Spanish (it's kinda like how "stamp" can have several words in Spanish, just wanna confirm with the language person that this is the correct/commonly used translation).
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.
@zarahzachz No, sadly, I don't speak Spanish. I popped this into google translate and got "Imprimir". Is there a language expert at CMS I could reach out to? I'm assuming I need to get Melissa's approval, but it would be nice to have it correct beforehand.
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.
I can ask Krithi who the language resource is because I always forget. Also, if you're doing multiple icons, probably best to batch the translation requests for all of them because it could take a little while to get a reply.
You shouldn't need Melissa's approval on this sorta stuff (design should have cleared the icon additions with her before it got assigned to code). At least that's how it worked for us. You'll probably wanna chat with your team to see if that's an expectation.
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.
Got a name: Von Portalatin (we've consulted with her for healthcare stuff in the past).
Might be good to specify that you're looking for the Spanish verbs for your icons (maybe that's specific enough for her? 🤷♀️)
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.
@zarahzachz Thanks! Was able to confirm with my team that "Imprimir" is correct.
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.
Another thing we can do when we need a CMS-approved translation but don't want to make a new request is to inspect product teams' translations or ask them for their translation for a particular pattern.
…from incons.stories.tsx
…CMSgov/design-system into tamara/wnmgds-2861-add-utility-icon
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.
LGTM!
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.
🎉
* Add PrintIcon component. * Removed fillColor attribute from PrintIcon componet and removed lint from incons.stories.tsx
* Initial file creation and addition of copy * Punctuation updates. * Updating snapshots * Revert "Updating snapshots" This reverts commit fd0f3bc. * [WNMGDS-2861] Adds PrintIcon component (#3196) * Add PrintIcon component. * Removed fillColor attribute from PrintIcon componet and removed lint from incons.stories.tsx * [WNMGDS-2878] Render Doc Site stories for Modal Dialog and Drawer (#3203) Copy changes and add ability to handle control arguments * Added print icon * [WNMGDS-2861] Adds Email, Link, RSS & What's New Icon components (#3202) * Adds Email and RSS icons. * Adds Link and What's New Icons * Exchange (') for curly (’) * Adding all icons and text * Making left icon boolean text explicitly relate to Figma * Ghost buttons and two columns * Addressed much feedback * Revise examples box * [NO-TICKET] Automatically format json files (#3206) * Run prettier on json files * Update lint-staged config to format json files * [WNMGDS-2875] Fix Release script take 2 (#3205) * Export vars and function needed in release script * Reorder imports and add new reviewers * Add new utility functions used by bumpVersions * Handle staging files properly * Export correct function from versions * export readJson so maybe we capture new line with read file' ' * big refactor add comments break up inline for loop and associated functions * Wrap removal of top level package json in try catch and uncomment woopsie * Add maybe more helpful comment * Further refactoring * Looks like a big refactor but basically moved all code to a new file * Adding functionality so we don't lose newline at end of json files * [NO-TICKET] Automatically format json files (#3206) * Run prettier on json files * Update lint-staged config to format json files * Undo new line addition when writing json * Fix typo * Move version number into our main function to make usage clearer --------- Co-authored-by: Patrick Wolfert <patrick.wolfert@adhocteam.us> * [RELEASE] 8/12 Version bump main (#3200) * Publish - @cmsgov/design-system@11.0.0-beta.0 - @cmsgov/ds-cms-gov@11.0.0-beta.0 - @cmsgov/ds-healthcare-gov@15.0.0-beta.0 - @cmsgov/ds-medicare-gov@13.0.0-beta.0 * [NO-TICKET] Manually updates "@cmsgov/design-system" version in package dependencies (#3201) manually updates dependency of "@cmsgov/design-system" version * Reverts design-system-tokens version back to 0.0.1 --------- Co-authored-by: Kim Niedermaier <kimniedermaier+cmsds@navapbc.com> Co-authored-by: Patrick Wolfert <patrick.wolfert@adhocteam.us> * [WNMGDS-2884] Add `cmsds-migrate` script for v11 CSS changes (#3208) * Basic migration script for v11 variable/class changes * These two variables weren't removed! We need to update our release notes, because that was a mistake * Fix syntax error (stray single quote) * Fix regex group not getting printed in the replacement text * Mention the migration script in the blog post and explain how to use it * [WNMGDS-2890] Fix `onAnalyticsEvent` attribute rendering to DOM through `HelpDrawer` (#3210) Fix `onAnalyticsEvent` prop bleeding into `dialog` DOM elements through `HelpDrawer` Without this fix, the unit test I added will print errors to the console like this: ``` console.error Warning: Unknown event handler property `onAnalyticsEvent`. It will be ignored. at dialog at NativeDialog (/Users/patrickwolfert/Projects/design-system/packages/design-system/src/components/NativeDialog/NativeDialog.tsx:38:3) at Drawer (/Users/patrickwolfert/Projects/design-system/packages/design-system/src/components/Drawer/Drawer.tsx:73:5) at HelpDrawer (/Users/patrickwolfert/Projects/design-system/packages/design-system/src/components/HelpDrawer/HelpDrawer.tsx:13:11) ``` * [NO-TICKET] Define a better return type for `defaultMenuLinks` (#3211) Be more informative about what `defaultMenuLinks` returns in the types Previously the return type was `{}`, which is useless to teams trying to use that functions themselves * [NO-TICKET] Revises release v11 blog post (#3213) update v11 blog post to remove reference to removing the privacy settings dialog * [NO-TICKET] Exclude core design system from dependency update (#3217) * filter out the core design system * Add comment and make conditional more readable * make conditional more readable * [WNMGDS-2713] Add default value overrides to Storybook (#3216) * Initial set of default values added to props tables * Updated storybook snaps * [WNMGDS-2838] Update all themes to make inline tool tips black (#3199) * Update all themes to make inline tool tips black * Udpate playwright vrt snapshots * Updating css snapshots * Updated css themes and custom tooltip scss styling to maybe no avail * setting tooltip color to match base color of surrounding text * Restore weirdly changed snapshots curious if CI browser tests pass * [WNMGDS-2875] Fix Release script take 2 (#3205) * Export vars and function needed in release script * Reorder imports and add new reviewers * Add new utility functions used by bumpVersions * Handle staging files properly * Export correct function from versions * export readJson so maybe we capture new line with read file' ' * big refactor add comments break up inline for loop and associated functions * Wrap removal of top level package json in try catch and uncomment woopsie * Add maybe more helpful comment * Further refactoring * Looks like a big refactor but basically moved all code to a new file * Adding functionality so we don't lose newline at end of json files * [NO-TICKET] Automatically format json files (#3206) * Run prettier on json files * Update lint-staged config to format json files * Undo new line addition when writing json * Fix typo * Move version number into our main function to make usage clearer --------- Co-authored-by: Patrick Wolfert <patrick.wolfert@adhocteam.us> * Undoing accidental hover styling change --------- Co-authored-by: Patrick Wolfert <patrick.wolfert@adhocteam.us> * Revert "Updating snapshots" This reverts commit fd0f3bc. * Addition of tabble and new content * Added content and table * Fixed table and aria labels --------- Co-authored-by: Tamara deMent (Corbalt) <tamara@corbalt.com> Co-authored-by: Patrick Wolfert <patrick.wolfert@adhocteam.us> Co-authored-by: Kim Niedermaier <kimniedermaier+cmsds@navapbc.com>
Summary
Adds a new
PrintIcon
component, one of six icons to be added from Jira ticket: WNMGDS-2861How to test
Run
yarn storybook
, go to Icons.Checklist
[WNMGDS-####] Title
or [NO-TICKET] if this is unticketed work.Type
(only one) label for this PR, if it is a breaking change, label should only beType: Breaking
Impacts
, multiple can be selected.