Skip to content

ModalPiPiP in mobile CallWithChat set to higher position and fixed bounds.#1748

Merged
mgamis-msft merged 17 commits intomainfrom
mgamis/move-modal-pip-higher-and-fix-bounds
Apr 6, 2022
Merged

ModalPiPiP in mobile CallWithChat set to higher position and fixed bounds.#1748
mgamis-msft merged 17 commits intomainfrom
mgamis/move-modal-pip-higher-and-fix-bounds

Conversation

@mgamis-msft
Copy link
Copy Markdown
Contributor

@mgamis-msft mgamis-msft commented Apr 1, 2022

What

Why

How Tested

Tested on local CallWithChat sample:
https://user-images.githubusercontent.com/79475487/161246430-2b33c2ab-abb5-4472-827e-b4692fd4ab9a.mp4
With rtl:
https://user-images.githubusercontent.com/79475487/161247324-74496b2f-7478-43c8-b399-7ffe37f584d6.mp4

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2022

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 1, 2022

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

</CallAdapterProvider>
);

const modalLayerHost = document.getElementById(props.modalLayerHostId);
Copy link
Copy Markdown
Member

@JamesBurnside JamesBurnside Apr 1, 2022

Choose a reason for hiding this comment

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

Can we instead use callWIthChatCompositeRootDiv.getElementById() instead of document?
If Contoso ever has more than one composite on the page this will break.
@dmceachernmsft recently added a reference to this rootDiv to do the same thing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tested this with two composites side by side and it works fine.
https://user-images.githubusercontent.com/79475487/161810214-6cdb126f-2042-4cc0-94a8-940c50e4d316.mp4
This is because the id of the modal layer host is uniquely generated in the CallWithChat composite with useId from '@fluentui/react-hooks'
I could change reference the CallWithChat composite div but it would be another prop that needs to be added to CallWIthChatPane.

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.

Oh nice! Nope what you have sounds good!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 5, 2022

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 5, 2022

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

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

github-actions Bot commented Apr 5, 2022

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 5, 2022

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 5, 2022

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 5, 2022

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 6, 2022

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 6, 2022

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 6, 2022

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 6, 2022

@mgamis-msft mgamis-msft merged commit ded9427 into main Apr 6, 2022
@mgamis-msft mgamis-msft deleted the mgamis/move-modal-pip-higher-and-fix-bounds branch April 6, 2022 19:58
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