Skip to content

[Stable blocking] Prevent showing screenShareButton when formFactor is set to mobile#3915

Merged
mgamis-msft merged 9 commits intomainfrom
mgamis/revise-bug-fix-to-hide-screenshare-button-for-iOS
Dec 14, 2023
Merged

[Stable blocking] Prevent showing screenShareButton when formFactor is set to mobile#3915
mgamis-msft merged 9 commits intomainfrom
mgamis/revise-bug-fix-to-hide-screenshare-button-for-iOS

Conversation

@mgamis-msft
Copy link
Copy Markdown
Contributor

What

Prevent showing screenShareButton when formFactor is set to mobile

Why

CallControlOptions screenShareButton was set to true by accident on sample app for all devices that are not iOS. We should prevent showing the screenShareButton when formFactor is set to mobile

How Tested

Local testing of Calling and CallwithChat samples on Desktop and Android

Process & policy checklist

  • I have updated the project documentation to reflect my changes if necessary.
  • I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • This change causes current functionality to break.

@mgamis-msft mgamis-msft changed the title Prevent showing screenShareButton when formFactor is set to mobile [Stable blocking] Prevent showing screenShareButton when formFactor is set to mobile Dec 14, 2023
@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 14, 2023

Calling bundle size is increased❗.

  • Current size: 5612288
  • Base size: 5612212
  • Diff size: 76

Comment thread samples/CallWithChat/src/app/views/CallScreen.tsx
callControls: {
screenShareButton: !shouldDisableScreenShare
}
callControls: shouldDisableScreenShare
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Avoid setting the whole callControls object based on shouldDisableScreenShare, makes it hard for someone to add options to this in future (they'd have to move the shouldDisableScreenShare)
Instead can we do:

callControls: {
  screenShareButton: shouldDisableScreenShare ? false : undefined
}

?

Copy link
Copy Markdown
Member

@JamesBurnside JamesBurnside Dec 14, 2023

Choose a reason for hiding this comment

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

(Or fix the shouldDisableScreenshare to be isMobile || isIOS instead of just isIOS)

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the composite UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the composite UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the composite UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the composite UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the composite UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the composite UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the composite UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the composite UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@mgamis-msft mgamis-msft enabled auto-merge (squash) December 14, 2023 21:46
@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@mgamis-msft mgamis-msft merged commit cf8225f into main Dec 14, 2023
@mgamis-msft mgamis-msft deleted the mgamis/revise-bug-fix-to-hide-screenshare-button-for-iOS branch December 14, 2023 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants