-
Notifications
You must be signed in to change notification settings - Fork 32
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
Import XFC as ES6 module #27
Conversation
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.
Thanks @pgross41 ! Could you update the README to include way to disable logs?
@kolkheang is this ready to be merged? |
@@ -4,9 +4,38 @@ In order to run any SMART app in Cerner's MPage view (iframe), the following pro | |||
|
|||
This library uses [xfc](https://github.com/cerner/xfc) to solve [Clickjacking](https://www.owasp.org/index.php/Clickjacking) problem for all browsers. | |||
|
|||
## Availability | |||
## Installation | |||
|
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.
Could you add a heading for NPM Module(Recommended)
README.md
Outdated
}; | ||
``` | ||
|
||
Warning: Disable XFC logging if using [F-Twelve](https://github.com/cerner/f-twelve/). F-Twelve writes to the DOM every time `console.log` is called and (when logging is enabled) XFC calls `console.log` every time the DOM is written to. This causes the browser to endlessly loop and freeze. It is safe to use both concurrently if XFC logging is disabled. |
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.
Perhaps bold the Warning: part
README.md
Outdated
import 'cerner-smart-embeddable-lib'; | ||
``` | ||
|
||
The minified & transpiled files are also available in the `dist/` directory and can be used with a `<script>` tag. |
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 could be under a new heading Script Include (NOT recommended) since you can't turn off XFC logging and it would conflict with MPages integration.
@pgross41 I just got a chance to review. Please let me know if the comments make sense. @shriniketsarkar could you take a look at this PR as well? |
@kolkheang made changes based on your comments please re-review |
@kolkheang is this ready to merge? |
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.
Looks good. @pgross41 Thanks for this PR.
#26