-
Notifications
You must be signed in to change notification settings - Fork 48
Add Mission Control Panel widget and mini-widget #2240
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
base: master
Are you sure you want to change the base?
Add Mission Control Panel widget and mini-widget #2240
Conversation
fc71ed4 to
90f1993
Compare
|
From a quick look through the code (haven't tested yet), it generally looks like it makes sense :-) I'm curious whether there's any special functionality in the main widget version of the control panel though - if not it should maybe only be available as a mini widget (which should already be able to go into a collapsible container if someone wants it in one)? In terms of state tracking, it could be nice to differentiate the "visited" waypoints (not just the current target). I'm not sure whether we'd consider it sufficient to just display everything prior to the target as visited, or if we think it's worth actually keeping track of which ones have been reported as visited (and potentially even count how many times)... I think it could be nice to display the "current target" waypoint as a target icon - possibly just adding an extra ring around it? As I understand it targets are a well-understood symbol, which should make it more immediately obvious what it means for that waypoint to be highlighted/different. |
I agree that having both the mini and regular widgets for certain features can feel redundant, but for Mission Planning there are two main reasons to keep both:
Agreed; Issue #2246 has been created to implement that.
“Targets” does refer to a location to go, but it also carries weapon and shooting related meanings, which I think doesn’t really fit the friendly tone we want our products to have. |
|
@ArturoManzoli can you rebase this PR? I want to resume its review. |
90f1993 to
af9374e
Compare
@rafaellehmkuhl Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice addition.
Three things to be worked on:
- The current WP indication is overflowing in the mini-widget.
- The current WP indication is not present in the regular widget.
- Since we are removing the controls from the map widget, I believe we should add a migration that adds the regular mission control widget by default in the views that have map widgets, probably on the bottom-right corner. It should be done only once, so needs a migration flag (something like
hasAddedMissionControlWidgetAfterMigration) in themigrations.tsfile.
| /** | ||
| * | ||
| */ | ||
| downloadMissionFromVehicle: () => Promise<void> | ||
| /** | ||
| * | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing docs.
e9307a4 to
c700775
Compare
Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
…n functions Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
…sion functions Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
…u skip-to-wp command
…widget and mini-widget Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
c700775 to
92b9c12
Compare
Couldn't reproduce here, but added some constrains so it won't happen again anyway. Could have been an issue specific to your OS.
Added that now!
Tried doing the migration inside the migrations.ts file, but it after speaking in private with @rafaellehmkuhl we decided that the best solution was to do the migrations inside the widgetManager store. |
Screenshare.-.2026-01-27.12_14_26.PM.mp4
Close #2032
Close #2119
Partially solves issue #2141