FEC-8696 ui components #399
Conversation
…e injected component from the player preact tree
esakal
requested review from
yairans and
OrenMe
and removed request for
yairans
| @@ -20,23 +20,20 @@ const mapStateToProps = state => ({ | |||
| config: state.config.components.vrStereo | |||
| }); | |||
|
|
|||
| const COMPONENT_NAME = 'vrStereo'; | |||
There was a problem hiding this comment.
move file to vr-stereo.js and also dir name
There was a problem hiding this comment.
need to fix the shouldRender method to get the component config by the new COMPONENT_NAME attribute as per our change of the disaplyName attribute - this might be a bug
There was a problem hiding this comment.
also, all component names start with capital, so please align, same for volume component
| /** | ||
| * should render component | ||
| * @param {*} props - component props | ||
| * @returns {boolean} - whether to render the component | ||
| * @static | ||
| */ | ||
| static shouldRender(props: any): boolean { | ||
| const componentConfig = props.config.components[this.displayName]; | ||
| const componentConfig = props.config.components['watermark']; |
There was a problem hiding this comment.
change this and add to convention doc for exposing config containers
| const componentConfig = props.config.components['watermark']; | |
| const componentConfig = props.config.components[COMPONENT_NAME.toLowerCase()]; |
need to change in other places too where this applies
There was a problem hiding this comment.
following our discussion won't do
| @@ -20,23 +20,20 @@ const mapStateToProps = state => ({ | |||
| config: state.config.components.vrStereo | |||
| }); | |||
|
|
|||
| const COMPONENT_NAME = 'vrStereo'; | |||
There was a problem hiding this comment.
need to fix the shouldRender method to get the component config by the new COMPONENT_NAME attribute as per our change of the disaplyName attribute - this might be a bug
| @@ -20,23 +20,20 @@ const mapStateToProps = state => ({ | |||
| config: state.config.components.vrStereo | |||
| }); | |||
|
|
|||
| const COMPONENT_NAME = 'vrStereo'; | |||
There was a problem hiding this comment.
also, all component names start with capital, so please align, same for volume component
src/components/volume/volume.js
Outdated
| @@ -23,38 +23,40 @@ const mapStateToProps = state => ({ | |||
| isMobile: state.shell.isMobile | |||
| }); | |||
|
|
|||
| const COMPONENT_NAME = 'volume'; | |||
There was a problem hiding this comment.
also, all component names start with capital, so please align, same for vrStereo component
allow adding/changing discrete components in UI related PRs: kaltura/playkit-js-ui#399 kaltura/kaltura-player-js#264
add ui components provided by plugins into the configuration object related PRs: kaltura/playkit-js#387 kaltura/playkit-js-ui#399
Description of the Changes
allow adding/changing discrete components in UI
related PRs:
kaltura/playkit-js#387
kaltura/kaltura-player-js#264
Missing implementations
CheckLists
This change is