Media Controls QuickView #123
Conversation
*Add media control quickview *Add signal in Arbiter for OpenAuto connection status change *Remove space in control bar between quickviews and power/close button Signed-off-by: Kyle <[email protected]>
Signed-off-by: Kyle <[email protected]>
| @@ -107,8 +107,6 @@ QWidget *Dash::control_bar() const | |||
| quick_views->setCurrentWidget(quick_view->widget()); | |||
| }); | |||
|
|
|||
| layout->addStretch(); | |||
There was a problem hiding this comment.
so removing this breaks the layout for the other quick views... I'm assuming you were trying to get everything centered on the control bar? The stretch was added here to have everything aligned to the left (to keep it distinct from the close+power buttons) so i think it should stay
since it looks like you're dealing with all the painting of the widget yourself, you'll probs need to handle sizing your widget (overriding sizeHint, etc) so the layout essentially knows to give it as much space as possible
or you might also be able to play with the layout in MediaQuickView::init (maybe)
There was a problem hiding this comment.
Yeah partly because I thought it looked better centered, and partly to give more room for the track details before it reverts to scrolling the information. I wasn't really sure why it was in there but I can certainly put it back and use a sizeHint to get the desired look without breaking the layout for other quick views.
There was a problem hiding this comment.
@rsjudka I've experimented with a couple different methods of changing the size of the media controls to override the default using sizeHints and sizePolicies and different stretch factors but nothing really works properly, it really wants to keep the default size of half the bar. I was thinking I could set it according to the parent size but the parent size changes after initialisation and the sizeHint is constant. The closest I could get was to set a huge width in the sizeHint so it would shrink into the proper width, but even then the height was hard to set right and wouldn't scale properly. None of this felt right either lol. IMO the proper approach would be to add a stretch inside of the quick_views where we want that separation from the power controls. Alternatively I could simply draw the prev, play, and next button at the far right of the area given to the Media Control, but the spacing won't be quite centred. The reason I'd personally prefer the former though is because I'm also working on a separate feature that would allow vehicle plugins to dynamically add quick views, which I think would be useful to plugin developers if they wanted to add, as an example, a HVAC control quick view for their particular vehicle. Having the full area available to developers would be desirable IMO. Let me know which approach you'd prefer to implement though, either one is fairly simple.
There was a problem hiding this comment.
oh yeah sorry my first recommendation would have been a bit more involved to get things sized for your quick view, but i think adding the ability to add quick views dynamically sounds awesome (and would definitely benefit from having the control bar's layout more adaptable)
as to not break the current quick views when this gets merged, could you go ahead and test moving the stretch to the individual quick views?
There was a problem hiding this comment.
Sorry to get back to you so late @rsjudka, life's been busy so I haven't had much time to work on this. I did implement and test the changes as stated and it works as intended. I'll try and get to pushing the changes today. I do have the dynamic quick views implemented in another local branch too, which I've been using for my particular application where I inject some HVAC controls from the vehicle's plugin. If you're interested I can merge those into this branch too, or I can create a separate PR, whichever you'd prefer.
There was a problem hiding this comment.
no worries :) i think as a separate pr would be great so we can get merged in as soon as youre ready with it!
| QString track_info(QString::fromStdString(metadata_.artist_name()) + | ||
| " - " + | ||
| QString::fromStdString(metadata_.track_name())); | ||
| QFont myFont("arial", 10); |
There was a problem hiding this comment.
id prefer if you used Session::Forge::font just to keep everything consistent and could have the size adjust based on a user's scale
There was a problem hiding this comment.
On the topic of scaling - is there going to be an issue with the hard coded constants I see?
There was a problem hiding this comment.
Will do! I'll also be sure to change it so the track information is centered according to the font metrics instead of the 5px offset like I have now. @icecube45 what do you mean by hard coded constants? Besides the font, I'm pretty sure everything is set in relation to the height and width of the widget, except the padding of course. If the user adjusts the scale, the height of the control bar changes because of the power buttons to the right, the MediaWidget draws the icons according to the height of the control bar so it adjusts to the scale automatically. I've attached some screenshots taken with a large and small scale applied. Let me know if I'm missing something else tho!
There was a problem hiding this comment.
I was referring to things like the QSize(128,128) or drawArc(play_rect,1440,5760);, but im not too concerned
There was a problem hiding this comment.
i was a bit concerned when i saw that first but it seems like everything is done relative to the widgets size so things should size appropriately (the qsize doesnt matter much here since the painter determines the size and those constants there are just for the arc's angle so all good there)
| aa_handler->injectButtonPress(aasdk::proto::enums::ButtonCode::PREV,openauto::projection::ButtonEventType::RELEASE); | ||
| } | ||
| if(play_rect.contains(event->pos())){ | ||
| aa_handler->injectButtonPress(aasdk::proto::enums::ButtonCode::TOGGLE_PLAY,openauto::projection::ButtonEventType::PRESS); |
There was a problem hiding this comment.
I thought I had made a helper method that sent both press and release events for you, but I can't find where that ended up (if it made it in at all), but might be something to double check
There was a problem hiding this comment.
there's the AAHandler::injectButtonPressHelper, is that what you're talking about?
There was a problem hiding this comment.
Mmm, no, that's for the event filter stuff.
*Add media control quickview
*Add signal in Arbiter for OpenAuto connection status change
*Remove space in control bar between quickviews and power/close button
Signed-off-by: Kyle [email protected]
Description:
Related issue (if applicable): fixes #
Checklist: