-
Notifications
You must be signed in to change notification settings - Fork 55
feat(cc-widgets): integ env #488
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
base: ccwidgets
Are you sure you want to change the base?
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a Vidcast showing the oAuth working in INT environment in both React & WC samples?
function toggleIntegrationEnvironment() { | ||
integrationEnvironment = integrationEnvironmentElem.checked; | ||
// Store in localStorage for persistence | ||
localStorage.setItem('integrationEnvironment', JSON.stringify(integrationEnvironment)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we bring the same localStorage setup in the react sample app as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no testing details added to the description.
Please add the vidcast and network logs
aria-label="integration environment checkbox" | ||
id="integration-environment-checkbox" | ||
label="Enable Integration Environment" | ||
// @ts-expect-error: TODO: https://github.com/momentum-design/momentum-design/pull/1118 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this comment here?
@@ -54,6 +54,8 @@ <h1>Contact Center widgets as web-component</h1> | |||
<br/> | |||
<input type="checkbox" id="theme" name="theme" /> Dark Theme | |||
<br /> | |||
<input type="checkbox" id="integrationEnvironment" name="integrationEnvironment" onchange="toggleIntegrationEnvironment()" /> Enable Integration Environment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: We don't have to say environment everywhere, just call it env
function toggleIntegrationEnvironment() { | ||
integrationEnvironment = integrationEnvironmentElem.checked; | ||
// Store in localStorage for persistence | ||
localStorage.setItem('integrationEnvironment', JSON.stringify(integrationEnvironment)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What advantage we have if we use localstorage to store this info and fetch it? Why can't we directly read the checkbox
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its not retaining after redirect from oauth
Couple of observations from the testing:
|
COMPLETES #Adhoc
This pull request addresses
Adding provision to login with integration environment in widgets samples
< DESCRIBE THE CONTEXT OF THE ISSUE >
by making the following changes
added check box in samples page to enable integration env
< DESCRIBE YOUR CHANGES >
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging