Add ionic range control #1148
Add ionic range control #1148
Conversation
| @@ -53,6 +54,7 @@ import { JsonFormsControl } from './control'; | |||
| export class JsonFormsOutlet extends JsonFormsBaseRenderer implements OnInit, OnDestroy { | |||
|
|
|||
| private subscription: Subscription; | |||
| private componentRef: ComponentRef<any>; | |||
There was a problem hiding this comment.
maybe improve naming eg by using 'currentComponentRef' ?
| const componentFactory = | ||
| this.componentFactoryResolver.resolveComponentFactory(bestComponent); | ||
| this.viewContainerRef.clear(); | ||
| const component: ComponentRef<any> = |
There was a problem hiding this comment.
maybe improve naming eg by using 'newComponentRef' /
| if (this.componentRef === undefined || | ||
| this.componentRef.componentType !== component.componentType) { | ||
| this.viewContainerRef.clear(); | ||
| this.componentRef = this.viewContainerRef.createComponent(componentFactory); |
There was a problem hiding this comment.
why can't we simply assign 'component' to 'this.componentRef' ?
packages/core/src/util/runtime.ts
Outdated
| const condition = uischema.rule.condition; | ||
|
|
||
| if (isLeafCondition(condition)) { | ||
| const value = resolveData(data, toDataPath(condition.scope)); |
There was a problem hiding this comment.
why are we using considtion.scope and not conditionScope here?
| RankedTester, | ||
| rankWith, | ||
| uiTypeIs | ||
| GroupLayout, JsonFormsProps, |
| @@ -305,9 +305,14 @@ export interface ControlState { | |||
| */ | |||
| export const mapStateToControlProps = | |||
| (state: JsonFormsState, ownProps: OwnPropsOfControl): StatePropsOfControl => { | |||
| const { uischema } = ownProps; | |||
There was a problem hiding this comment.
after extracting the uischema we should use it everywhere
packages/core/src/util/renderer.ts
Outdated
| const path = composeWithUi(ownProps.uischema, ownProps.path); | ||
| const visible = _.has(ownProps, 'visible') ? ownProps.visible : isVisible(ownProps, state); | ||
| const enabled = _.has(ownProps, 'enabled') ? ownProps.enabled : isEnabled(ownProps, state); | ||
| const rulePath = _.has(ownProps.uischema, 'rule') |
There was a problem hiding this comment.
I assume you fixed this because of a bug?
if so we should add tests for this.
how can I reproduce the bug?
There was a problem hiding this comment.
It's related to the ListWithDetail renderer where a scope is set and a detail view with a rule has been specified as well. The rule's scope is based on a schema ref, but actually should be depenent on an instance path because of array indices. I added a test case demonstrating this.
edgarmueller
force-pushed
the
topic/ionic/range-control
branch
from
c9c3ba8
to
908da9f
Compare
edgarmueller
force-pushed
the
topic/ionic/range-control
branch
from
908da9f
to
6110013
Compare
|
LGTM, thank you |
Furthermore:
componentReflazily initialized