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

feat(Icon): Icons for screen sharing control in the calling screen #1490

Merged

Conversation

jay-howe
Copy link
Contributor

@jay-howe jay-howe commented Jun 11, 2019

Problem description
Please port the new ubar icons for screen sharing control to StarDust
image

@codecov
Copy link

codecov bot commented Jun 11, 2019

Codecov Report

Merging #1490 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1490      +/-   ##
==========================================
+ Coverage   73.44%   73.45%   +0.01%     
==========================================
  Files         818      822       +4     
  Lines        6179     6183       +4     
  Branches     1793     1793              
==========================================
+ Hits         4538     4542       +4     
  Misses       1636     1636              
  Partials        5        5
Impacted Files Coverage Δ
.../svg/ProcessedIcons/icons-call-control-release.tsx 100% <100%> (ø)
...s/components/Icon/svg/icons/callControlRelease.tsx 100% <100%> (ø)
...s/components/Icon/svg/icons/callControlRequest.tsx 100% <100%> (ø)
.../svg/ProcessedIcons/icons-call-control-request.tsx 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 840a3ea...5c30079. Read the comment docs.

@alinais
Copy link
Contributor

alinais commented Jun 12, 2019

#1485

@alinais alinais changed the title #1485 Icons for screen sharing control in the calling screen feat(Icon): Icons for screen sharing control in the calling screen Jun 12, 2019
@kuzhelov
Copy link
Contributor

@codepretty, could you, please, take a look and verify SVG paths/viewport size/name for the introduced icon? Thank you!

@@ -150,6 +152,8 @@ export default {
'call-parking': callParking,
'call-video': callVideo,
'call-video-off': callVideoOff,
'call-control-release': callControlRelease,
Copy link
Contributor

@kuzhelov kuzhelov Jun 12, 2019

Choose a reason for hiding this comment

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

lets ensure those are pasted in alphabetical order to the set of other ones

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed to keep 'concepts-based' order

@@ -12,6 +12,8 @@ import bullets from './bullets'
import calendar from './calendar'
import call from './call'
import callParking from './callParking'
import callControlRelease from './callControlRelease'
Copy link
Contributor

Choose a reason for hiding this comment

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

lets maintain alphabetical order, to simplify search scenarios

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am keeping release and request as a group (viewer) as they are most closely related. The other call-control-xxx icons are a different group (sender)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have grouped them by concepts as opposed to lexical order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The existing list is already out of order. Do you want me to fix it?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, lets stick to the order pattern already established - thank you for clarification!

Copy link
Contributor

@kuzhelov kuzhelov left a comment

Choose a reason for hiding this comment

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

please, just add changelog entry

@kuzhelov kuzhelov merged commit 7b3e624 into microsoft:master Jun 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants