-
Notifications
You must be signed in to change notification settings - Fork 4
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: Integrated Optimizely Prompt Experiment #52
Changes from 9 commits
2fb1862
3412f8f
8482d7b
62aaec8
58691b9
bd16de8
8d96749
7e6a5a1
7867ea2
cb402b8
c055843
03ce47e
03b500a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,19 +30,21 @@ const Disclosure = ({ children }) => ( | |
</ul> | ||
</div> | ||
</div> | ||
<p className="disclaimer text-light-100 py-3"> | ||
<strong>Note: </strong> | ||
This chat is AI generated (powered by ChatGPT). Mistakes are possible. | ||
By using it you agree that edX may create a record of this chat. | ||
Your personal data will be used as described in our | ||
<Hyperlink | ||
className="privacy-policy-link text-light-100" | ||
destination={getConfig().PRIVACY_POLICY_URL} | ||
> | ||
privacy policy | ||
</Hyperlink> | ||
. | ||
</p> | ||
{getConfig().PRIVACY_POLICY_URL ? ( | ||
<p className="disclaimer text-light-100 py-3"> | ||
<strong>Note: </strong> | ||
This chat is AI generated (powered by ChatGPT). Mistakes are possible. | ||
By using it you agree that edX may create a record of this chat. | ||
Your personal data will be used as described in our | ||
<Hyperlink | ||
className="privacy-policy-link text-light-100" | ||
destination={getConfig().PRIVACY_POLICY_URL} | ||
> | ||
privacy policy | ||
</Hyperlink> | ||
. | ||
</p> | ||
) : null } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This solves a warning in the tests. Since The full warning was:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since it seems like this config value is effectively required for this component maybe we want to make use of ensureConfig()? https://github.com/openedx/frontend-platform/blob/b0ac45329f0ed9022541abea4fe30abf49fde0b1/src/config.js#L264 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed. I wasn't familiar with all the config options. Updated and it's much cleaner now. Thanks for the suggestion. |
||
{children} | ||
</div> | ||
); | ||
|
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.
This reduces CI checks log file size from 30KB to 8KB by silencing warnings.
At some point warnings should be reduced, but we are not taking them into account on the CI, so we just might just remove them.
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.
Also, some warnings are caused by external libraries, like
@openedx/paragon
icons for example. When testing, the library renders the icons as<IconMock />
by the use of a string reference rather than a component causing the following warning:This warning stacks up for every render.