Skip to content
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

fix(composer): selectedDataBinding not able to update selected node #274

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

sheilaXu
Copy link
Contributor

Overview

when selectedDataBinding is set before the scene is loaded, the findSceneNodeRefBy function will always find nothing (because nodes are not loaded yet) and then set selected node to undefined.

setting camera target before components rendered will not work as expected because the bounding box calculated will be incorrect.

Legal

This project is available under the Apache 2.0 License.

@lgtm-com
Copy link

lgtm-com bot commented Oct 11, 2022

This pull request fixes 1 alert when merging 258b22c2be12900d83d10a52e59565923f33cc8d into 9c1b9e6 - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@sheilaXu sheilaXu marked this pull request as ready for review October 11, 2022 21:22
@lgtm-com
Copy link

lgtm-com bot commented Oct 11, 2022

This pull request fixes 1 alert when merging 389038b8abaa843eb432930482ad3c92767a95d8 into 9c1b9e6 - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Oct 11, 2022

This pull request fixes 1 alert when merging 9d84f7d into 9c1b9e6 - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

Copy link
Contributor

@jwills-jdubs jwills-jdubs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is truly no way to handle this all within state manager I suppose that is alright. But you need to ensure activeCamera is not affected by this as well and I am afraid this could complicate things.

if (isEqual(prevSelectedRef.current, props.selectedDataBinding)) {
return;
}

prevSelectedRef.current = props.selectedDataBinding;

if (props.selectedDataBinding === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a lot of this stuff is done in the wrong place hence the issue with it not being loaded. I think you should consider putting it where we handle active camera instead and try to have it be handled by the state manager

https://github.com/awslabs/iot-app-kit/blob/main/packages/scene-composer/src/components/StateManager.tsx#L102

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the selectedDataBinding feature is currently a viewer only feature and is not tested with editing mode at all. so I think it's more clear to be in this component at least for now.
also, no matter this logic is here or in state manager, we will have the same not loaded issue.

@sheilaXu
Copy link
Contributor Author

If there is truly no way to handle this all within state manager I suppose that is alright. But you need to ensure activeCamera is not affected by this as well and I am afraid this could complicate things.

setting active camera is tested and working

@sheilaXu sheilaXu merged commit b23bce2 into main Oct 11, 2022
@sheilaXu sheilaXu deleted the fix-selected-data-binding branch October 11, 2022 22:38
TheEvilDev added a commit that referenced this pull request Nov 2, 2022
* Release 1.2.1 (#85)

* fix: unsubrscribe data provider on component updates

* Release 1.2.1

Co-authored-by: Norbert Nader <nnader@amazon.com>

* fix: resolves #83 (#87)

* fix: resolves #83

* Update index.ts

Co-authored-by: Norbert Nader <nnader@amazon.com>

* feat: improve documentation (#90)

* Run tests on pull request (#91)

* Update reference to code name (#86)

AWS-UI internal code name is mentioned in the readme.

Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com>

* feat: update synchro charts to 3.1.0, update docs (#92)

* feat: Improve documentation formatting (#93)

* feat: Improve resource explorer docs (#95)

* Update README.md (#94)

* feat: support fetchMostRecentBeforeStart (#79)

* Update documentation & make react-components easier to use (#98)

* Utilize lock file in CI (#99)

* build(deps): bump async from 2.6.3 to 2.6.4 (#100)

Bumps [async](https://github.com/caolan/async) from 2.6.3 to 2.6.4.
- [Release notes](https://github.com/caolan/async/releases)
- [Changelog](https://github.com/caolan/async/blob/v2.6.4/CHANGELOG.md)
- [Commits](caolan/async@v2.6.3...v2.6.4)

---
updated-dependencies:
- dependency-name: async
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: support auto-assigning colors for certain components (#96)

* feat: support auto-assigning colors for certain components

* Update package.json

* Update cypress to v9, added stricted linting, fixed linting warnings and errors (#104)

* Add stylelint and enforce via global package.json (#106)

* use most recent synchro-charts version, 4.0.1 (#111)

* build(deps): bump eventsource from 1.1.0 to 1.1.1 (#116)

Bumps [eventsource](https://github.com/EventSource/eventsource) from 1.1.0 to 1.1.1.
- [Release notes](https://github.com/EventSource/eventsource/releases)
- [Changelog](https://github.com/EventSource/eventsource/blob/master/HISTORY.md)
- [Commits](EventSource/eventsource@v1.1.0...v1.1.1)

---
updated-dependencies:
- dependency-name: eventsource
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: expand children in resource explorer (#115)

* feat: expand children in resource explorer

* fix typo

* Fix asset module cache

* Add logic for fetching hierarchies

* Add tests

* Improve insert

* Fix linting

Co-authored-by: Norbert Nader <nnader@amazon.com>

* Update README.md (#121)

Fix test runner status badge

* Bump minor version (#120)

* specify react types package version to be 17 (#134)

Co-authored-by: Xinyi <xinyxu@amazon.com>

* build(deps): bump parse-url from 6.0.0 to 6.0.2 (#142)

Bumps [parse-url](https://github.com/IonicaBizau/parse-url) from 6.0.0 to 6.0.2.
- [Release notes](https://github.com/IonicaBizau/parse-url/releases)
- [Commits](https://github.com/IonicaBizau/parse-url/commits)

---
updated-dependencies:
- dependency-name: parse-url
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fix: Development Quick Start Docs/Scripts (#148)

* add yarn bootstrap script to replace lerna bootstrap

* specify supported node and yarn engines

* simplify development docs and update build steps

Co-authored-by: Thomas Juranek <thjurane@amazon.com>

* build(deps): bump moment from 2.29.2 to 2.29.4 (#149)

Bumps [moment](https://github.com/moment/moment) from 2.29.2 to 2.29.4.
- [Release notes](https://github.com/moment/moment/releases)
- [Changelog](https://github.com/moment/moment/blob/develop/CHANGELOG.md)
- [Commits](moment/moment@2.29.2...2.29.4)

---
updated-dependencies:
- dependency-name: moment
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: update @testing-library/react to use create root (#151)

Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com>

* feat(props): adapt props for synchro-charts (#133)

* feat(props): adapt props for synchro-charts

* add testing for new props

* fix(tests): cleanup tests from devwork

* Update changelog

* update tests to wait for intercepts

Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com>

* feat: batch API for historical, aggregated, and latest value data (#137)

* fix: resolve float decimal precision issue on round() function. (#160)

* fix: update @testing-library/react to use create root

* fix: resolve float decimal precision issue on round() function.

* Simplify round() function. Remove managing the negative sign separately.

Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com>

* sync changes with private repo for common files (#150)

* sync changes with private repo for common files

* Revert "fix: update @testing-library/react to use create root (#151)"

This reverts commit 960f53f.

Co-authored-by: Xinyi <xinyxu@amazon.com>

* Update npm packages repository (#161)

* build(deps): bump terser from 4.8.0 to 4.8.1 (#168)

Bumps [terser](https://github.com/terser/terser) from 4.8.0 to 4.8.1.
- [Release notes](https://github.com/terser/terser/releases)
- [Changelog](https://github.com/terser/terser/blob/master/CHANGELOG.md)
- [Commits](https://github.com/terser/terser/commits)

---
updated-dependencies:
- dependency-name: terser
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* docs: Fix KPI docs and explain behavior of querying timer-series data when resolution is omitted

* bump synchro-charts/core version from v5 to v6 (#199)

Co-authored-by: Thomas Juranek <thjurane@amazon.com>

* docs: Add coding guidelines

* organize coding guidelines into sections and reword items (#212)

sentance casing on headings

Co-authored-by: Thomas Juranek <thjurane@amazon.com>

* feat(core)!: Refactor time series data module to  remove unused functionality. Add meta field to data stream.

BREAKING CHANGE: Refactored export from `@iot-app-kit/core` IoTAppKitDataModule to be named TimeSeriesDataModule, and removed the concept of multiple data sources per time series data module

* feat(core): Support caching of dataType, name and other fields describing dataStreams

* feat(core,source-iotsitewise)!: Change time series data modules getRequestsFromQueries to be async

Fix various eslint errors on existing code

* chore: remove unecessary template section

* build: npm workspaces (#231)

* feat(core): Add meta field to requestInfos in TimeSeriesDataModule

* build: npm publish with npm workspaces (#233)

* docs: add documentation for publishing new packages (#236)

* docs: add documentation around data sources and time series data

* feat(core): Add viewportManager to orchestrate viewport syncing within groups

* docs(core): Add documentation for viewportManager; export viewportManager from pkg

* feat: set up table package and add createTableItems method (#124)

- config typescript & rollup
- config jest
- config eslint
- add unit tests for createTableItems method
- updated .eslintrc.js at project root level

fix: update tsconfig path in eslintrc for Table package.

- remove unused variables.

fix: add tests for dataFilters.ts

- update descriptions for tests in createTableItems.spec.ts
- add one test case for getting data point from aggregated datastream

fix: remove unnecessary describe block

fix: use object to replace CellItem class
- add createCellItem function
- add unit tests for createCellItem
- rename SC_DataStream to SynchroChartsDataStream for better consistency

fix: remove unused import
- update createCellItem tests.

* feat(table): create new Table component based on AWSUI Table component (#129)

feat: add helper functions for

- iconUtils.tsx - copied from synchro-charts with changes.
- iconUtils.spec.ts - copied from synchro-charts with changes:
-- add one test cases, that color are provided, compared with the original.
- spinner.tsx - copied from synchro-charts with adjustments for React
- spinner.spec.tsx - copied from synchro-charts with adjustments for React

feat: add the main table component

fix: remove generic types for TableProps

fix: add size to loading spinner

fix: remove unnecessary TS compiler options

update a few things based on comments from PR.

Fix: remove full file eslint disable. Switch to minimal eslint disable.

* fix(table): fix an issue when key in columnDefinition doesn't exist in TableItems (#146)

fix: remove unused variables.

* feat!(component): replace table component in @iot-app-kit/components (#147)

* feat: Replace iot-table with new version.

* Update autogenerated types

* fix import

* another fix for import

* another fix for import

* Remove unused property for Iot.

* Fix a typo

* Code cleanups.

* Code cleanups.

* update type of `ColumnDefinition.formatter`

also added a few use cases in testing-ground.tsx

* update type of `ColumnDefinition.formatter`

also added a few use cases in testing-ground.tsx

* Update asset/property IDs and table examples.

* update: add formatter to filterOptions

Property filtering options now has the same displayed value as table.

* update formatPropertyFilterOptions function to better handle error & loading states.

* update test examples for table component.

* Disable Customized formatter for property filter options.

* update test cases after disabled formatter for filter options

* Remove unused dependency.

* a few bug fixes
- Split sorting and propertyFiltering
- Remove formatting on property filtering options.

* UI adjustment - center icon and string vertically.

* Update styles and unit tests

* Remove unused imports and statements.

* Reformat css file

* Restore testing ground

* chore(table): Merge changes from mainline and other changes: (#169)

* docs(table): update docs and add README.md (#165)

* Update docs. Add README.md for new table component.

* fix(table): add missing brackets. (#183)

* docs(table): update docs / major version bump for breaking changes.(#213)

* feat(table): support messageOverrides (#211)

* chore(table): add missing tests (#215)
- render correct data based on different viewports
- render spinner when a datastream is loading
- render error icon and message when a datastream in error state
- render icon and apply styles when it breaches thresholds

* docs(table): update docs / remove unnecessary jest configuration (#217)

* fix(components): move message merge to componentWillRender()

- clean git history and update doc
- removed unused method in `iot-react-table.tsx`

* build: add table to workspace

- updated docs
- export `dataFilters` from core and removed duplications from table.
- update npm-publish.yml
- update table styles
- iot-table: watch messageOverrides for changes

* docs(table): revert version bumps and doc changes.

* chore: update changelogs & docs / major version bumps to 2.0.0

* fix(components): import missing CSS style sheets.

* doc: update link in Coding Guidelines

* feat(TwinMaker): Add TwinMaker packages (#252)

* build(deps): bump scss-tokenizer from 0.3.0 to 0.4.3

Bumps [scss-tokenizer](https://github.com/sasstools/scss-tokenizer) from 0.3.0 to 0.4.3.
- [Release notes](https://github.com/sasstools/scss-tokenizer/releases)
- [Commits](sasstools/scss-tokenizer@v0.3.0...v0.4.3)

---
updated-dependencies:
- dependency-name: scss-tokenizer
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* doc(TwinMaker): update TwinMaker docs and link them to the main doc (#256)

* chore: update changelogs & version bump (#257)

* chore(components): reduce concurrent workers in testing

* Fixes the Camera Component Editor for Lens and Clipping planes updates when changing between cameras (#263)

* chore(examples): easier linking for iot-app-kit packages (#265)

* feat(components): support viewport grouping in iot-table using ViewportManager.

* feat(EnvironmentModel): Adding support for environment overlay (#262)

Add the EnvironmentModelComponent to adi in possible updates for loading of model later as well as offload responsibility from ModelRefComponent
Adds a menu item to the add object menu to add at most 1 environment model
Removes Sub model Tree from Environment models

Add more restrictions to the Environment nodes.
- Environment Nodes cannot have children
- Environment Nodes have no Transform Editor
- Environment Nodes cannot have Transform controls
- Environment Nodes cannot have a model shader
- Environment controls cannot have children

Add tests

* Fix: Updating documentation for Storybook (#264)

Co-authored-by: Emily Dodds <dodemily@amazon.com>

* chore(CameraView): Removes the dependency on the feature flag for the top bar camera drowpdown (#267)

This will improve the following:

1. No longer require a feature flag so no special build of grafana is required once feature ships
2. If there are no cameras we don't show the UI as it is confusing
3. This restores not showing the View Options if there are no Motion Indicators
4. Hides the top bar if neither exist for more screen real estate

* fix(core): add currentTime param to viewportStartDate and `viewportEndDate`

* fix(react-components, videoPlayer): fix propertiesNotChanged for videoPlayer

* chore(TagResize): Add flag to resizable tags (#270)

* Iotroci 8819 (#271)

* Fix: Refactor to improve ViewCursorWidget performance

* CR Feedback

* cleanup

* Fix from running npm run build

Co-authored-by: Emily Dodds <dodemily@amazon.com>

* Fix: Allows user to hit Esc key to cancel Enhanced Editing (#272)

Authored-by: Emily Dodds <dodemily@amazon.com>

* fix(composer): selectedDataBinding not able to update selected node (#274)

* doc: add TwinMaker components to GettingStarted doc (#275)

Co-authored-by: Xinyi Xu <xinyxu@amazon.com>

* fix(composer): use arrow data as texture to avoid public path issue (#276)

* fix(composer): fix show svg (#279)

* chore: code cleanup (#278)

* fix(composer): fix adding tag always attached to root (#281)

* fix: Fix the tag scaling with parent scale regression (#282)

* Fix the tag scaling with parent scale regression

* Re-add the change to handl the submodel ref as a parent

* fix(SceneHierarchyPerformance): SceneHierarchy Tree Performance (#283)

- Reduces the number of rerenders significantly in the SceneHierarchy Tree view whenever actions like activating, selecting and removing happen
- Removes redundent On2 loop when computing children for a scene hierarchy node
- Cleans up some new responsibilities for TreeItem and Tree Label that were added in the wrong places.

Co-authored-by: Mitchell Lee <mitchlee@amazon.com>

* fix(CameraView): Minimum FOV check introduced (#284)

* feat: Introduce alarms (#135)

* Fix the camera zoom field locked to 1 or greater (#286)

* Add success popup message when updating camera location (#285)

* Update the text from Cameras in the drop down to Camera View (#287)

* fix(composer): fix model shader material color restore (#290)

* Chore: CSS updates to Scene Hierarchy (#289)

* Fix: CSS updates to Scene Hierarchy

* Eslint fix

* Improving test coverage

* Eslint fixes for new test coverage

Co-authored-by: Emily Dodds <dodemily@amazon.com>

* Release 2.2.0 (#292)

* fix(composer): Fixes 2nd camera viewing click bug (#291)

* fix(composer): Fixes 2nd camera viewing click to look at object rather than view.

* Update the package.json test branch numbers due to auto-ratchet

Co-authored-by: Emily Dodds <mumanity@gmail.com>

* fix(composer):Camera and Light helper visibility toggling (#294)

* fix(deps): update synchro-charts, node-sass, nth-check (#295)

* chore: fix internal build (#300)

* chore: setup release-please (#301)

* fix(composer): update translations (#302)

Co-authored-by: Xinyi Xu <xinyxu@amazon.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com>
Co-authored-by: Norbert Nader <nnader@amazon.com>
Co-authored-by: db <77755322+diehbria@users.noreply.github.com>
Co-authored-by: Mitchell Lee <mitch@evildev.net>
Co-authored-by: Bowei Han <boweih@amazon.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Xinyi Xu <sheilaxxy@gmail.com>
Co-authored-by: Xinyi <xinyxu@amazon.com>
Co-authored-by: Thomas Juranek <thomas@juranek.com>
Co-authored-by: Thomas Juranek <thjurane@amazon.com>
Co-authored-by: Xiao(Bill) Li <limxiao@amazon.com>
Co-authored-by: jmbuss <107281089+jmbuss@users.noreply.github.com>
Co-authored-by: Brian Diehr <diehbria@amazon.com>
Co-authored-by: Jonathan Wills <65614034+jwills-jdubs@users.noreply.github.com>
Co-authored-by: Emily Dodds <mumanity@gmail.com>
Co-authored-by: Emily Dodds <dodemily@amazon.com>
Co-authored-by: Mitchell Lee <mitchlee@amazon.com>
Co-authored-by: Matt Popovich <mattpopovich@outlook.com>
@github-actions github-actions bot mentioned this pull request Nov 2, 2022
@github-actions github-actions bot mentioned this pull request Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants