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: render cta for unsupported content #2663

Merged
merged 48 commits into from
Oct 12, 2023
Merged

Conversation

musale
Copy link
Contributor

@musale musale commented Aug 15, 2023

Closes #2190

PR Type

  • Feature

Description of the changes

Renders a CTA component showing the content is unsupported and you need to use the main window to view it.

PR checklist

  • Project builds (yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)
  • All public APIs (classes, methods, etc) have been documented following the jsdoc syntax
  • Stories have been added and existing stories have been tested
  • Added appropriate documentation. Docs PR:
  • License header has been added to all new source files (yarn setLicense)
  • Contains NO breaking changes

Other information

@musale musale added this to the Chat - Public Preview milestone Aug 15, 2023
@musale musale requested a review from a team as a code owner August 15, 2023 10:32
@microsoft-github-policy-service
Copy link
Contributor

Thank you for creating a Pull Request @musale.

This is a checklist for the PR reviewer(s) to complete before approving and merging this PR:

  • I have verified a documentation PR has been linked and is approved (or not applicable)
  • I have ran this PR locally and have tested the fix/feature
  • I have verified that stories have been added to storybook (or not applicable)
  • I have tested existing stories in storybook to verify no regression has occured
  • I have tested the solution in at least two browsers (Edge + 1 non-Chromium based browser)

@musale musale linked an issue Aug 15, 2023 that may be closed by this pull request
Signed-off-by: Musale Martin <[email protected]>
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Author Feedback Issue needs response from issue author label Aug 16, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs: Author Feedback Issue needs response from issue author label Aug 22, 2023
gavinbarron
gavinbarron previously approved these changes Sep 6, 2023
packages/mgt-chat/src/components/Chat/Chat.tsx Outdated Show resolved Hide resolved
@sebastienlevert
Copy link
Contributor

A couple of comments. Might be related to design / specs that were a bit unclear. Let me know if we prefer managing them as part of this PR or as part of a set of bugs:

"body": {
                "contentType": "html",
                "content": "<hr><p>gdffdgdfg</p><p>dfg</p><h1>dfgdfgdfg</h1><p>dfgd</p><p style=\"margin-left:40px\">fgdfgdfgdfg</p><p>dfgdfg</p><p>&nbsp;</p><p>&nbsp;</p><table itemprop=\"copy-paste-table\"><tbody><tr><td><p>&nbsp;</p></td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr><tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr><tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr><tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr><tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr></tbody></table><p>&nbsp;</p><p>&nbsp;</p>"
            }

@musale
Copy link
Contributor Author

musale commented Sep 26, 2023

@sebastienlevert I have updated the first 2 issues. As for the rendering as plain text one, that is as a result of the MessageThreads component not currently supporting a way for customizing the ouput when you have unsupported content. It is a dirty hack that uses renderToString from react to generate a html string of the UnsupportedContent component. Then the html string is what is rendered as a chat message. An ideal solution would be something like what is implemented for onRenderAvatar where we could substitute what is originally used for unsupported content with our UnsupportedContent

@sebastienlevert
Copy link
Contributor

@musale Not sure I'm following on the third bullet. I would think that formatted HTML should be treated as valid and supported content. So we shouldn't render at all the UnsupportedContent component, right? Are we too greedy in the validation of what's supported or not?

@github-actions
Copy link

github-actions bot commented Oct 9, 2023

🚀 New react-contoso sample application deployed here

@sebastienlevert
Copy link
Contributor

The auto-deployment of React Contoso on any PR is ABSOLUTELY FANTASTIC! Thanks for making this a thing @gavinbarron!

@sebastienlevert
Copy link
Contributor

sebastienlevert commented Oct 10, 2023

Couple of things I saw, but might be related to the order of things.

@musale
Copy link
Contributor Author

musale commented Oct 11, 2023

@sebastienlevert for point 1, they will be rendered as mgt person when that PR is merged. For point 2, I narrowed it down to:

  • inserting a link.
  • inserting a table.

I have fixed the issue and should be good to go.

@github-actions
Copy link

🚀 New react-contoso sample application deployed here

@gavinbarron
Copy link
Member

@musale I see that you have resolved the comments about all of the extra eslint suppressions but I'm not seeing that they have been removed. Is there a commit missing from this change?

@github-actions
Copy link

🚀 New react-contoso sample application deployed here

@musale
Copy link
Contributor Author

musale commented Oct 12, 2023

@musale I see that you have resolved the comments about all of the extra eslint suppressions but I'm not seeing that they have been removed. Is there a commit missing from this change?

Yes. Here it is 7da8a5f

@github-actions
Copy link

🚀 New react-contoso sample application deployed here

@gavinbarron gavinbarron enabled auto-merge (squash) October 12, 2023 17:08
@github-actions
Copy link

🚀 New react-contoso sample application deployed here

@gavinbarron gavinbarron merged commit 3c00710 into next/mgt-chat Oct 12, 2023
8 checks passed
@gavinbarron gavinbarron deleted the unsupported-content branch October 12, 2023 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[mgt-chat] Rendering unsupported content
3 participants