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.

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/orr/custom icon preproccessor #293

Merged
merged 3 commits into from
Oct 24, 2021

Conversation

orrgottlieb
Copy link
Contributor

Go over this checklist before submitting your Pull Request

Description: ADD YOUR PR DESCRIPTION HERE

STEP 1: Fulfill the description of the PR above

STEP 2: Select one of 2 main scenarios of adding the new component or editing the existing one

STEP 3: Delete checklist which you don't need

STEP 4: Go over the checklist. All checkboxes should be marked

STEP 5: Delete this instruction

STEP 6: Happy code review and merge!

Adding new component

Basic

  • Used plop (npm run plop) to create a new component. This command will create a folder for your component and will update index.js file
  • PR has description
  • New component is functional and using React Hooks.
  • New React Hooks were added to /src/hooks folder (optional)
  • Component defines PropTypes
  • There is Readme for the component, which explains the main aspects of how to use it

Style

  • Styles are added to NewComponent.scss file inside of the NewComponent folder
  • CSS selectors are named using BEM convention
  • Design is compatible with Monday Design System

Storybook

  • Stories were added to /src/NewComponent/stories/NewComponent.stories.js file
  • Stories include sandbox story with knobs for each property
  • Stories include all flows of using the component
  • Component passed Accessibility Plugin checks

Tests

  • Tests are compliant with TESTING_README.md instructions

Updating existing component

Basic

  • PR has description
  • Changes to the component are backward compatible (including selectors structure). If not - add to the title of the PR "BREAKABLE_CHANGE""
  • All changes to the component are reflected in the ReadMe
  • If component is old and was not compliant with the latest guidelines - it was fixed (optional)

Style

Storybook

  • All the changes to the component should be reflected in the Storybook.
  • Component passed Accessibility Plugin checks

Tests

  • All current tests are passing
  • New functionality is covered with tests
  • Tests are compliant with TESTING_README.md instructions

import PropTypes from "prop-types";
import cx from "classnames";
import SVG from "react-inlinesvg";
import "./CustomSvgIcon.scss";
import useIconScreenReaderAccessProps from "../../hooks/useIconScreenReaderAccessProps";

const CustomSvgIcon = ({ className, src, onClick, clickable, ariaLabel, ariaHidden, ...props }) => {
function modifySvgCode(svg, color = "currentColor") {
return svg.replace(/fill=".*?"/g, `fill="${color}"`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to replace stroke as well?

*/
useCurrentColor: PropTypes.bool,
/**
* If you want to override to coloring of currentColor
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* If you want to override to coloring of currentColor
* If you want to override the icon's color

ariaHidden: PropTypes.bool
ariaHidden: PropTypes.bool,
/**
* when using svg from src (Icon.type.SRC) this boolean will transform the "fill" property to "currentColor"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* when using svg from src (Icon.type.SRC) this boolean will transform the "fill" property to "currentColor"
* when using svg from src (`Icon.type.SRC`) this boolean will make the icon inherit its color from text

@orrgottlieb orrgottlieb merged commit adc8878 into master Oct 24, 2021
@orrgottlieb orrgottlieb deleted the feat/orr/custom-icon-preproccessor branch October 24, 2021 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants