Skip to content

Commit

Permalink
fix: Issue with Opacity Rule didn't allow changing it's value (#100)
Browse files Browse the repository at this point in the history
removing unnecessary console.logs

Co-authored-by: Mitchell Lee <mitchlee@amazon.com>
  • Loading branch information
TheEvilDev and Mitchell Lee committed Sep 14, 2022
1 parent 9020c2f commit 420391c
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 36 deletions.
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/scene-composer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@
"lines": 75.88,
"statements": 74.98,
"functions": 75.7,
"branches": 61.51,
"branches": 61.5,
"branchesTrue": 100
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/scene-composer/src/components/StateManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ const StateManager: React.FC<SceneComposerInternalProps> = ({
if (config.featureConfig) {
setFeatureConfig(config.featureConfig);
}
}, [config.featureConfig]);
}, [config]);

useEffect(() => {
if (config.metricRecorder) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const DebugInfoPanel = () => {
return (
<ExpandableInfoSection
title={formatMessage({ defaultMessage: 'Debug Info', description: 'Section title' })}
defaultExpanded
defaultExpanded={false}
>
<FormField label={formatMessage({ defaultMessage: 'Ref', description: 'Form field label' })}>
<Input disabled value={selectedSceneNode?.ref || ''}></Input>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ import React from 'react';
import { useIntl, defineMessages } from 'react-intl';
import { Grid, Select } from '@awsui/components-react';

import { SceneResourceType } from '../../../interfaces';
import { COMPOSER_FEATURES, SceneResourceType } from '../../../interfaces';
import {
convertToIotTwinMakerNamespace,
getSceneResourceDefaultValue,
getSceneResourceInfo,
} from '../../../utils/sceneResourceUtils';
import useFeature from '../../../hooks/useFeature';

import { SceneRuleTargetColorEditor } from './SceneRuleTargetColorEditor';
import { SceneRuleTargetIconEditor } from './SceneRuleTargetIconEditor';
Expand Down Expand Up @@ -44,10 +45,16 @@ export const SceneRuleTargetEditor: React.FC<ISceneRuleTargetEditorProps> = ({
const targetInfo = getSceneResourceInfo(target);
const { formatMessage } = useIntl();

const options = Object.values(SceneResourceType).map((type) => ({
label: formatMessage(i18nSceneResourceTypeStrings[SceneResourceType[type]]) || SceneResourceType[type],
value: SceneResourceType[type],
}));
const [{ variation: opacityRuleEnabled }] = useFeature(COMPOSER_FEATURES[COMPOSER_FEATURES.OpacityRule]);

const options = Object.values(SceneResourceType)
.filter((type) => {
return opacityRuleEnabled === 'C' ? type !== SceneResourceType.Opacity : true;
})
.map((type) => ({
label: formatMessage(i18nSceneResourceTypeStrings[SceneResourceType[type]]) || SceneResourceType[type],
value: SceneResourceType[type],
}));
return (
<Grid gridDefinition={[{ colspan: 4 }, { colspan: 8 }]}>
<Select
Expand Down Expand Up @@ -81,7 +88,7 @@ export const SceneRuleTargetEditor: React.FC<ISceneRuleTargetEditorProps> = ({
onChange={(targetValue) => onChange(convertToIotTwinMakerNamespace(targetInfo.type, targetValue))}
/>
)}
{targetInfo.type === SceneResourceType.Opacity && (
{opacityRuleEnabled === 'T1' && targetInfo.type === SceneResourceType.Opacity && (
<SceneRuleTargetOpacityEditor
targetValue={targetInfo.value}
onChange={(targetValue) => onChange(convertToIotTwinMakerNamespace(targetInfo.type, targetValue))}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback } from 'react';
import React, { useCallback, useState } from 'react';
import { Input, Grid } from '@awsui/components-react';

interface ISceneRuleTargetOpacityEditorProps {
Expand All @@ -10,16 +10,29 @@ const SceneRuleTargetOpacityEditor: React.FC<ISceneRuleTargetOpacityEditorProps>
targetValue,
onChange,
}: ISceneRuleTargetOpacityEditorProps) => {
const onInputChange = useCallback(
({ target }) => {
onChange(target.value);
},
[onChange],
);
const [val, setVal] = useState(targetValue);

const onInputChange = useCallback(({ detail, target }) => {
let { value } = detail || target;
value = value >= 1 ? 1 : value; // disable user from going out of range
value = value <= 0 ? 0 : value;
setVal(value);
}, []);

const onBlur = useCallback(() => {
onChange(val);
}, [val, onChange]);

return (
<Grid gridDefinition={[{ colspan: 9 }, { colspan: 3 }]}>
<Input data-testid={'tm-opacity-field'} value={targetValue} onChange={onInputChange} />
<Input
data-testid={'tm-opacity-field'}
type='number'
step={0.01}
value={val}
onChange={onInputChange}
onBlur={onBlur}
/>
</Grid>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { render, fireEvent } from '@testing-library/react';
import SceneRuleTargetOpacityEditor from '../SceneRuleTargetOpacityEditor';

jest.mock('@awsui/components-react', () => ({
Grid: 'grid',
Grid: 'Grid',
Input: 'input',
}));

Expand All @@ -27,7 +27,9 @@ describe('<SceneRuleTargetOpacityEditor>', () => {

const input = await findByTestId('tm-opacity-field');

fireEvent.focusIn(input);
fireEvent.change(input, { target: { value: '0.5' } });
fireEvent.focusOut(input);

expect(onChange).toBeCalledWith('0.5');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ exports[`<SceneRuleTargetOpacityEditor> render expected field 1`] = `
>
<input
data-testid="tm-opacity-field"
step="0.01"
type="number"
value="1"
/>
</grid>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import React, { useMemo, useEffect } from 'react';
import { Mesh } from 'three';

import { SceneResourceType } from '../../../interfaces';
import { COMPOSER_FEATURES, SceneResourceType } from '../../../interfaces';
import { ISceneNodeInternal, IColorOverlayComponentInternal, useStore } from '../../../store';
import { useSceneComposerId } from '../../../common/sceneComposerIdContext';
import { getSceneResourceInfo, parseColorWithAlpha } from '../../../utils/sceneResourceUtils';
import useMaterialEffect from '../../../hooks/useMaterialEffect';
import useRuleResult from '../../../hooks/useRuleResult';
import useFeature from '../../../hooks/useFeature';

interface IColorOverlayComponentProps {
node: ISceneNodeInternal;
Expand All @@ -20,6 +21,7 @@ const ColorOverlayComponent: React.FC<IColorOverlayComponentProps> = ({
const { ruleBasedMapId, valueDataBinding } = component;
const sceneComposerId = useSceneComposerId();
const entityObject3D = useStore(sceneComposerId)((state) => state.getObject3DBySceneNodeRef(node.ref));
const [{ variation: opacityRuleEnabled }] = useFeature(COMPOSER_FEATURES[COMPOSER_FEATURES.OpacityRule]);

const ruleResult = useRuleResult(ruleBasedMapId!, valueDataBinding!);

Expand Down Expand Up @@ -59,13 +61,9 @@ const ColorOverlayComponent: React.FC<IColorOverlayComponentProps> = ({
if (ruleResult !== '') {
transform();
}
}, [ruleResult]);

useEffect(() => {
return () => {
restore();
};
}, []);
return () => restore();
}, [ruleResult, entityObject3D, opacityRuleEnabled]);

// This component relies on side effects to update the rendering of the entity's mesh. Returning an empty fragment.
return <React.Fragment></React.Fragment>;
Expand Down
1 change: 1 addition & 0 deletions packages/scene-composer/src/hooks/useFeature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { getGlobalSettings, subscribe, unsubscribe } from '../common/GlobalSetti
const useFeature = (feature: string) => {
const state = getGlobalSettings().featureConfig[feature];
const [featureState, setFeatureState] = useState(state);
console.log(getGlobalSettings().featureConfig);

const onUpdated = () => {
const newState = getGlobalSettings().featureConfig[feature];
Expand Down
18 changes: 9 additions & 9 deletions packages/scene-composer/src/interfaces/feature.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
export enum COMPOSER_FEATURES {
FOR_TESTS, // Feature flags may come and go, as things launch, this one is used for automated testing, so please don't remove it.
i18n,
SceneHierarchyRedesign,
SceneHierarchySearch,
SceneHierarchyReorder,
SceneHierarchyMultiSelect,
SubModelSelection,
OpacityRule,
ENHANCED_EDITING,
FOR_TESTS = 'FOR_TESTS', // Feature flags may come and go, as things launch, this one is used for automated testing, so please don't remove it.
i18n = 'i18n',
SceneHierarchyRedesign = 'SceneHierarchyRedesign',
SceneHierarchySearch = 'SceneHierarchySearch',
SceneHierarchyReorder = 'SceneHierarchyReorder',
SceneHierarchyMultiSelect = 'SceneHierarchyMultiSelect',
SubModelSelection = 'SubModelSelection',
OpacityRule = 'OpacityRule',
ENHANCED_EDITING = 'ENHANCED_EDITING',
}

export type FeatureConfig = Partial<Record<COMPOSER_FEATURES, boolean>>;
1 change: 1 addition & 0 deletions packages/scene-composer/stories/SceneComposer.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ WaterTank.args = {
[COMPOSER_FEATURES.SceneHierarchyReorder]: true, // Drag/Drop Reordering
[COMPOSER_FEATURES.SubModelSelection]: true,
[COMPOSER_FEATURES.ENHANCED_EDITING]: true,
[COMPOSER_FEATURES.OpacityRule]: true,
},
},
};
Expand Down

0 comments on commit 420391c

Please sign in to comment.