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

Multi Vehicle Panel Overhaul #11970

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Conversation

demirciAhmet
Copy link
Contributor

@demirciAhmet demirciAhmet commented Oct 6, 2024

Description

Related to #11693

Here are the changes by the panel overhaul:

  • New solid top-right panel implemented, changed overall design of the multi vehicle list.
  • To be able to add guided action confirmation to the buttons of the multi-vehicle panel, a new logic "Select vehicles - Perform action" implemented (now you first select vehicles from the panel by tapping on them, then perform the action to the selected vehicles, otherwise you had to confirm one by one for the each action, which would be very annoying).
  • Guided action confirmation for the mv actions
  • New swipe pages embedded in the new top-right panel. This way, other components such as the PhotoVideoControl, or any other widgets that may be implemented later, can be added to the top-right panel.

image

untitled.1.mp4

Checklist:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@DonLakeFlyer
Copy link
Contributor

Is there a fixed amount of space you devote to vehicle indicators which scrolling? is this why the list is clipped at the bottom?

@demirciAhmet
Copy link
Contributor Author

Is there a fixed amount of space you devote to vehicle indicators which scrolling? is this why the list is clipped at the bottom?

Yes, it has a fixed height (currently half of the panel height), you scroll through the panel if there are many vehicles. This way there is no need to worry about adding new widgets to the panel in the future.

I thought about separating the lower part of the panel for buttons, and other widgets, which will be stacked page by page.

@demirciAhmet demirciAhmet marked this pull request as ready for review October 29, 2024 18:20
@demirciAhmet demirciAhmet changed the title [Draft] Multi Vehicle Panel Overhaul Multi Vehicle Panel Overhaul Oct 29, 2024
@HTRamsey HTRamsey requested a review from DonLakeFlyer October 30, 2024 22:04
@demirciAhmet demirciAhmet force-pushed the panel_overhaul branch 7 times, most recently from 68f6768 to f4b191d Compare January 12, 2025 08:52
@DonLakeFlyer
Copy link
Contributor

DonLakeFlyer commented Jan 13, 2025

Some initial problems from a quick look...

When there are no vehicles connected there is an artifact left on the screen in the upper right:
Screenshot 2025-01-13 at 8 34 54 AM

When I am connected to multiple vehicles I see that same artifact and there is nothing that indicates there is a multi-vehicle panel available. If I click on the oval thing the panel finally opens up.

Problems with the panel itself:
Screenshot 2025-01-13 at 8 35 51 AM

  • The panel itself is much too large. It should only take as much horizontal/vertical space as needed with minimal borders. That way it will work on android tablet sized screens. Target for tablet is a 10" screen.
  • The panel overlaps with the HUD indicators as you can see in the screen shot. It should only be as tall as the space available from below the toolbar to the top of the HUD. Then it should scroll up/down if needed.

Copy link
Contributor

@DonLakeFlyer DonLakeFlyer left a comment

Choose a reason for hiding this comment

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

So far this is looking really good. Thanks. Biggest problems are real estate problems.

src/Vehicle/MultiVehicleManager.cc Show resolved Hide resolved
src/FlightDisplay/MultiVehicleList.qml Outdated Show resolved Hide resolved
@demirciAhmet
Copy link
Contributor Author

When there are no vehicles connected there is an artifact left on the screen in the upper right: Screenshot 2025-01-13 at 8 34 54 AM

When I am connected to multiple vehicles I see that same artifact and there is nothing that indicates there is a multi-vehicle panel available. If I click on the oval thing the panel finally opens up.

Actually, the idea on that point was that the panel itself, which you can open, close, and use even when connected to a single vehicle, should indicate the availability of a multi-vehicle panel. However, as you mentioned earlier, most users operate QGC with a single vehicle, so this might not be the best design approach.

Perhaps the most suitable option would be to convert the panel into something hidden by default but accessible by swiping from the right side of the screen, similar to sidebar menus on Android. When multiple vehicles are connected, the panel will automatically show itself while still allowing users to open and close it via swiping. What do you think?

@DonLakeFlyer
Copy link
Contributor

Here is what I would do:

  • The visibility of the multi-vehicle panel is controlled through a setting which is part of FlyViewSettings. It defaults to visible.
  • Add that as a setting in the Settings / Fly View / General ui
  • If the setting in set to visible still only show the panel when multiple vehicles are available
  • Rework the active vehicle selector in the toolbar. Make it look more like flight mode selection dropdown instead of a combo. In the expanded page for the dropdown, put the multi-vehicle panel on/off setting in there as well

How about that?

@demirciAhmet
Copy link
Contributor Author

Here is what I would do:

* The visibility of the multi-vehicle panel is controlled through a setting which is part of FlyViewSettings. It defaults to visible.

* Add that as a setting in the Settings / Fly View / General ui

* If the setting in set to visible still only show the panel when multiple vehicles are available

* Rework the active vehicle selector in the toolbar. Make it look more like flight mode selection dropdown instead of a combo. In the expanded page for the dropdown, put the multi-vehicle panel on/off setting in there as well

How about that?

So, you're suggesting we stick with something similar to the previous approach regarding the on/off point. But it might result in a loss of flexibility and make it harder to use?

Isn't this "while still allowing users to open and close it via swiping" approach more user friendly?

@DonLakeFlyer
Copy link
Contributor

My thinking was along the lines of you either use the panel or you don't use the panel. If you don't use the panel then you don't want stuff no matter how small cluttering things up. I was not thinking that you might hide/show the panel multiple times during the same flight. In that case, yes some quicker way to access it would be better. But I'm not sure if that an important use case to consider?

I don't want to create new paradigms for the UI unless they are really necessary. And then if something new is added it is added in a consistent way. For example if you can swipe this multi-vehicle thing off the screen. Then why can't you swipe the video thing off the screen when you don't need it and are just flying a single vehicle. If it's not a critical need then I'd rather have consistency in how things are done.

@demirciAhmet
Copy link
Contributor Author

My thinking was along the lines of you either use the panel or you don't use the panel. If you don't use the panel then you don't want stuff no matter how small cluttering things up. I was not thinking that you might hide/show the panel multiple times during the same flight. In that case, yes some quicker way to access it would be better. But I'm not sure if that an important use case to consider?

I don't want to create new paradigms for the UI unless they are really necessary. And then if something new is added it is added in a consistent way. For example if you can swipe this multi-vehicle thing off the screen. Then why can't you swipe the video thing off the screen when you don't need it and are just flying a single vehicle. If it's not a critical need then I'd rather have consistency in how things are done.

Oh, I see what you mean now—that makes sense. I’ll probably complete the improvements tomorrow. Should I push the changes as a new commit for easier review, or rebase again and force push?

@DonLakeFlyer
Copy link
Contributor

The setting for the panel in the toolbar indicator should use the same wording as in Fly View Settings: "Enable Multi-Vehicle Panel". Also it should be in the expanded portion of the indicator which matches the usage of settings in indicators in all the other indicators.

@DonLakeFlyer
Copy link
Contributor

Took a look. Perfect, thanks. This is great new work. I'll merge once it passes CI.

@demirciAhmet
Copy link
Contributor Author

Took a look. Perfect, thanks. This is great new work. I'll merge once it passes CI.

Glad to hear! Thank you too.

@DonLakeFlyer DonLakeFlyer requested review from DonLakeFlyer and removed request for DonLakeFlyer January 17, 2025 22:04
@DonLakeFlyer DonLakeFlyer merged commit d32d5f4 into mavlink:master Jan 17, 2025
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.

2 participants