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

chore(dial): port over DialBase from SynchroCharts #590

Merged
merged 1 commit into from
Feb 16, 2023
Merged

chore(dial): port over DialBase from SynchroCharts #590

merged 1 commit into from
Feb 16, 2023

Conversation

diehbria
Copy link
Contributor

Overview

Ports over dial component from synchrocharts. Also refactors to remove calls to calculate bounding boxes and eliminate a lot of state mutations, in preference for a simpler, more straightforward implementation

Follow up PR will be made which will add icons, tests, etc. This is the first iteration, and does not expose the dial component publicly.

Legal

This project is available under the Apache 2.0 License.

@diehbria diehbria marked this pull request as ready for review February 16, 2023 03:10
});
const { viewport } = useViewport();

const utilizedViewport = passedInViewport || viewport; // explicitly passed in viewport overrides viewport group
Copy link
Contributor

Choose a reason for hiding this comment

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

Should an override be a part of the viewport hook? I imagine every component is going to have this line?

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'm not so sure, while your right it'll be a common pattern, it is possibly more intuitive for people to just handle this themselves then have to understand that a viewport passed into useViewport overrides the viewport and makes it so use viewport does nothing

@diehbria diehbria merged commit 90a0790 into main Feb 16, 2023
@diehbria diehbria deleted the dials branch February 16, 2023 17:39
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