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

feat: Refactor SiteWise components to use query and provider interfaces #54

Merged
merged 1 commit into from
Feb 11, 2022

Conversation

boweihan
Copy link
Contributor

Overview

Move away from the current "bag of props" component interface. Components now accept a generic Query interface. Any queries that conform to the interface are guaranteed to "provide" the component with the data it needs to render. Think of them as generic data connectors that can be easily reimplemented!

For SiteWise components we also handle gestures from SynchroCharts that result in viewport updates. This custom logic is the responsibility of our SiteWiseTimeSeriesDataProvider.

Tests

https://github.com/boweihan/iot-app-kit/actions/runs/1825938466

Legal

This project is available under the Apache 2.0 License.

this.provider = this.query.build(this.appKit.session(this.widgetId), this.defaultSettings);
}

@Listen('dateRangeChange')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not build and listen in the connector component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this for two reasons:

  1. I wanted to separate the presentational concerns (App kit component) from data fetching concerns (iot time series connector). dateRangeChange is a synchro-charts specific functionality so it should belong in the "presentational" layer - users should be able to implement their own presentational components on-top of iot-time-series-connector, for example. I was also thinking there might be a case where we want to handle viewport updates differently depending on the type of presentational component.
  2. The appKit session has an optional componentId and I figured it made sense to pass in widgetId directly in these base charts rather than having users define their own (this is why we call build() in this class).

Copy link
Contributor

Choose a reason for hiding this comment

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

This makes perfect sense. Thank you!

@boweihan boweihan merged commit dd24732 into awslabs:main Feb 11, 2022
sheilaXu pushed a commit that referenced this pull request Sep 23, 2022
* Updated the function definitions for getAvailableTimeRanges(), triggerLiveVideoUpload() and triggerOnDemandVideoUploadRequest()
* Removed unnecessary try-catch blocks
This was referenced 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.

3 participants