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

Settings API #35

Merged
merged 7 commits into from
Aug 21, 2024
Merged

Settings API #35

merged 7 commits into from
Aug 21, 2024

Conversation

FraLab09
Copy link
Contributor

@FraLab09 FraLab09 commented Jul 1, 2024

User-Facing Changes

Description
Allow the user to link settings to message converters depending on the panel type. The settings are assigned directly in the topics part of the foxglove settings by using the fromSchemaName property set in the converter
Here's an extension to test that functionality: it basically adds a setting for the em/drivingtube/polyline topic in the 3D panel to change the shape of the displayed dots from cubes to spheres

Checklist

  • The web version was tested and it is running ok
  • The desktop version was tested and it is running ok
  • I've updated/created the storybook file(s)
  • The release version was updated on package.json files

Copy link
Contributor

@aneuwald-ctw aneuwald-ctw left a comment

Choose a reason for hiding this comment

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

Hey @FraLab09, it seems that some CI steps failed, could you have a look on those? As I saw, it is only small stuffs, like import libraries and not use them. Thanks!

@laisspportugal
Copy link
Contributor

Can you also please add some unit tests? At the moment I think it's better to wait till all CI checks are running ok to do a better review on our side, cause the code will be probably modified.
Thanks in advance!

@aneuwald-ctw aneuwald-ctw self-requested a review July 10, 2024 12:34
Copy link
Contributor

@aneuwald-ctw aneuwald-ctw left a comment

Choose a reason for hiding this comment

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

lodash-es will be addressed later

packages/studio-base/src/PanelAPI/useConfigById.ts Outdated Show resolved Hide resolved
@aneuwald-ctw aneuwald-ctw dismissed their stale review July 10, 2024 12:41

Already solved

@laisspportugal
Copy link
Contributor

@laisspportugal
Copy link
Contributor

Looks good to me, thanks for your effort

Copy link
Contributor

@aneuwald-ctw aneuwald-ctw left a comment

Choose a reason for hiding this comment

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

LGTM

@aneuwald-ctw aneuwald-ctw merged commit bd4f626 into Lichtblick-Suite:main Aug 21, 2024
8 checks passed
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.

5 participants