-
Notifications
You must be signed in to change notification settings - Fork 64
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: support ability for consumers to add custom props / HTML attributes defined via configuration to components #723
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #723 +/- ##
==========================================
+ Coverage 83.41% 83.89% +0.48%
==========================================
Files 40 41 +1
Lines 1073 1105 +32
Branches 197 210 +13
==========================================
+ Hits 895 927 +32
Misses 166 166
Partials 12 12 ☔ View full report in Codecov by Sentry. |
e5e6bcc
to
be3418f
Compare
example/PiiPage.jsx
Outdated
<h3><code>withPii</code> (HOC)</h3> | ||
<p> | ||
<i>Username:</i>{' '} | ||
<UsernameWithPii2 ref={usernameRef}> |
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.
[inform] Shows example passing a ref
that should be forwarded.
be3418f
to
122023a
Compare
d3a37c4
to
148934c
Compare
8bfc4ea
to
263841c
Compare
…es HOC for custom props
50344a8
to
5b53e4c
Compare
@@ -629,7 +629,7 @@ exports.publish = (memberData, opts, tutorials) => { | |||
generateSourceFiles(sourceFiles, opts.encoding); | |||
} | |||
|
|||
// if (members.globals.length) { generate('Global', [{kind: 'globalobj'}], globalUrl); } | |||
if (members.globals.length) { generate('Global', [{kind: 'globalobj'}], globalUrl); } |
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.
[inform] generates the global.html
file which is used in links for custom @typedef
types used within the React module. without this, the generated JSDoc link results in a 404. that said, the globals still do not show in the sidebar navigation.
Closed in favor of openedx/frontend-plugin-framework#84 |
Description
Context
Open edX instances (e.g., 2U) rely on third-party tools for logging and analytics (e.g., Datadog, Hotjar). Some of these tools end up ingesting PII into things such as session replays, etc. that should otherwise be masked.
Example vendor-specific approaches to masking UI elements from their ingested data:
data-dd-privacy="allow" | "mask" | "hidden" | "mask-user-input"
.data-hj-suppress
anddata-hj-allow
..fs-mask
,.fs-unmask
,.fs-exclude
, etc. class names.Currently, such attributes for Hotjar are hardcoded throughout the platform (search results), despite not all instances of Open edX using Hotjar. While these attributes are fairly benign, the current approach does not support extending the masking of PII to other vendors (e.g., Datadog).
Related, there are other vendor-specific HTML attributes and class names unrelated to PII this approach would also be applicable to. As such, the following implementation is generic to support "custom props" on annotated elements, regardless of whether it's for PII masking or otherwise.
Solution
Introduces a React hook (
useComponentPropOverrides
) and a Higher-Order-Component (withComponentPropOverrides
) as a mechanism to allow consumers to add one or more custom props (e.g., to mask PII within session replays) to any component that accepts prop spreading (e.g.,...props
).Configuration
May be configured by
env.config.js
or the MFE runtime configuration withcomponentPropOverrides
(open to other config property names):Usage
Consider the following JSX containing PII (i.e., username):
useComponentPropOverrides
(hook)withComponentPropOverrides
(HOC)By using
useComponentPropOverrides
(hook) orwithComponentPropOverrides
(HOC) to mask the username from third-party tools with the abovecomponentPropOverrides
configuration, the resulting HTML would render as the following:It does this by attempting to find a match between the component's specified selector and the
componentPropOverrides.targets.*
defined via configuration. If the specified selector matches one of the selectors defined via configuration, any configured props and their values be returned.Special cases
By default, components supporting configurable prop overrides only works with the prop
className
and any prop prefixed withdata-
(e.g.,data-dd-privacy
). Any other configuration prop name is ignored and not applied during rendering.In certain cases, components may opt-in to supporting overrides of explicitly named prop names (e.g.,
onClick
). By making the special cases like function handlers orstyle
opt-in within the MFE's base code, it provides opportunity to discuss/review which prop overrides are officially supported in any given component beyond the defaultdata-*
attributes orclassName
prop.className
Custom class name(s) will be concatenated with any existing
className
prop values that might already be passed to the component.style
Custom
style
properties will be shallow merged with any existingstyle
properties that might already exist for the component. If the style properties overlap, the customstyle
's value takes precedence.Functions (e.g.,
onClick
)If a custom prop is defined as a function (i.e., supported when
customComponentProps
is defined viaenv.config
) but the component itself already has anonClick
function handler, the component'sonClick
function will be executed first before calling the customonClick
handler. Both functions otherwise receive the same functionsargs
.Merge checklist:
frontend-platform
. This can be done by runningnpm start
and opening http://localhost:8080.module.config.js
file infrontend-build
.fix
,feat
) and is appropriate for your code change. Consider whether your code is a breaking change, and modify your commit accordingly.Post merge: