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

styleSettings API introduction #38

Merged
merged 1 commit into from
Jan 24, 2022
Merged

styleSettings API introduction #38

merged 1 commit into from
Jan 24, 2022

Conversation

diehbria
Copy link
Contributor

@diehbria diehbria commented Jan 24, 2022

Overview

Implementation of styleSettings, which uses the refId present in data streams to provide presentational settings to components.

Refactored the api from styles -> styleSettings, as stencil ignores the prop styles. Likely is a reserved keyword that you cannot pass an object into.

Legal

This project is available under the Apache 2.0 License.

packages/core/src/data-module/IotAppKitDataModule.spec.ts Outdated Show resolved Hide resolved
@@ -37,7 +41,7 @@ export class IotConnector {
request: this.request,
},
(dataStreams: DataStream[]) => {
this.dataStreams = dataStreams;
this.dataStreams = bindStylesToDataStreams({ dataStreams, styleSettings: this.styleSettings });
Copy link
Contributor

Choose a reason for hiding this comment

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

(random thoughts) It initially felt strange to me to bind style to dataStreams, since styles are metadata and not streamed. I see that it follows the synchro-chart API though, and these are essentially DataStreamStyles. Would we ever want to swap out synchro-charts as a viz library? If not then this approach seems fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how we bind styleSettings will be component specific, right now iot-connector is a bit specific to Synchro Charts, but we can refactor it as necessary as the need arises as iot-connector is purely an internal helper and not intended to be used as part of the offerings of the library

boweihan
boweihan previously approved these changes Jan 24, 2022
Copy link
Contributor

@boweihan boweihan left a comment

Choose a reason for hiding this comment

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

:shipit:

@diehbria diehbria force-pushed the styles-types branch 2 times, most recently from 1cf3b19 to 73b7b07 Compare January 24, 2022 22:18
@diehbria diehbria marked this pull request as draft January 24, 2022 22:21
@diehbria diehbria marked this pull request as ready for review January 24, 2022 22:26
@diehbria diehbria merged commit 7b82989 into main Jan 24, 2022
@diehbria diehbria deleted the styles-types branch January 24, 2022 23:48
sheilaXu added a commit that referenced this pull request Sep 23, 2022
Co-authored-by: Xinyi Xu <xinyxu@amazon.com>
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.

2 participants