Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

feat(Icons): Added icon for add-page and also added missing icon sizes #601

Merged
merged 9 commits into from
Dec 12, 2018

Conversation

priyankar205
Copy link
Collaborator

  1. Added add-page icon
  2. Added sizes 'x-large' and 'xx-large' for icons.

@kuzhelov kuzhelov added the needs author feedback Author's opinion is asked label Dec 12, 2018
@@ -18,4 +18,5 @@ export default {
</svg>
),
styles: {},
exportedAs: 'canvas-add-page',
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -50,6 +51,7 @@ export default {
'call-control-present-new': callControlPresentNew,
'call-control-stop-presenting-new': callControlStopPresentingNew,
'call-recording': callRecording,
'canvas-add-page': canvasAddPage,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@kuzhelov kuzhelov added ready for merge and removed needs author feedback Author's opinion is asked labels Dec 12, 2018
@kuzhelov kuzhelov merged commit 820ca9a into master Dec 12, 2018
@kuzhelov kuzhelov deleted the priyankar/add-icons branch December 12, 2018 16:14
if (!sizeModifier) {
return sizes.get(size)
}
const modifiedSizes = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the context this icon will be used? We will need a way to specify sizing, but I don't believe we want to do it in css. In Teams, the different sizes should actually be different icons (in most cases).

@levithomason
Copy link
Member

We need to undo the complexity added here with the sizeModifier. The code base already has number sizes, Semantic UI sizes, plus an additional 'micro' size added. This PR introduces a 4th concept to sizing.

We should instead unify our 3 sizing patterns to accomplish this feature. Let's always reduce complexity with changes.

@levithomason
Copy link
Member

#136 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants