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: add pie chart plugin #315

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

yukkysaito
Copy link

@yukkysaito yukkysaito commented Dec 24, 2024

User-Facing Changes

Thank you for maintaining Lichtblick.

I have created a plugin to display a float32 array as a pie chart. I developed it based on the Gauge plugin and used Recharts as the foundation. However, I’m not yet familiar with Lichtblick's coding practices, so the current implementation is just at a functional level. Could you kindly provide support on what would be needed to make it suitable for merging?

Additionally, the Gauge plugin I referenced has recently undergone significant refactoring in this PR. The Pie Chart plugin was developed based on the pre-refactored version.
Description
image

Screencast.from.2024.12.10.22.49.54.webm

I plan to organize the commit history later.

Checklist

  • The web version was tested and it is running ok
  • The desktop version was tested and it is running ok
  • This change is covered by unit tests
  • Files constants.ts, types.ts and *.style.ts have been checked and relevant code snippets have been relocated

yukkysaito and others added 13 commits December 8, 2024 01:00
Signed-off-by: Yukihito Saito <[email protected]>
Signed-off-by: Yukihito Saito <[email protected]>
Signed-off-by: Yukihito Saito <[email protected]>
Signed-off-by: Yukihito Saito <[email protected]>
Signed-off-by: Yukihito Saito <[email protected]>
Signed-off-by: Yukihito Saito <[email protected]>
Signed-off-by: Yukihito Saito <[email protected]>
Signed-off-by: Yukihito Saito <[email protected]>
Signed-off-by: Yukihito Saito <[email protected]>
Signed-off-by: Yukihito Saito <[email protected]>
@luluiz
Copy link
Member

luluiz commented Dec 26, 2024

Hi @yukkysaito,
Thank you in advance for your contribution.
As a team, we'll discuss this contribution to follow up on your PR.
Regarding Lichtblick's coding practices, we recently started improving the practices applied to the project, and these improvements are currently in progress based on some approved ADRs.

The team will get back to you soon.

// file, You can obtain one at http://mozilla.org/MPL/2.0/

// SPDX-FileCopyrightText: Copyright (C) 2024 Yukihiro Saito <[email protected]>
// SPDX-License-Identifier: Apache-2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please verify the license? Since we use MPL-2.0 why do you think is necessary to change to Apache?

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment applies to all files

@laisspportugal
Copy link
Contributor

laisspportugal commented Jan 8, 2025

Hello @yukkysaito

We discussed this as a team and will evaluate your PR, leaving some comments for you.
We have already enabled the pipeline checks. Could you please review the errors? One tip is to run yarn install and then commit the changes to theyarn.lock file.

Best Regards,
Lichtblick team

@yukkysaito
Copy link
Author

@luluiz @laisspportugal Thank you for your comment. I was on a business trip overseas until recently, so my reply was delayed.
I will address each matter in order, so please wait a little longer.

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.

4 participants