[vlc-devel] [PATCH] Qt4: UI enhancements

[Jean-Baptiste Kempf]

On Sat, Dec 31, 2011 at 10:18:16AM -0500, Edward Wang wrote :
> +            TimeLabel *timeLabel = new TimeLabel( p_intf, TimeLabel::Both );

I think this should be the default and therefore should have a
default constructor without the argument.

> +    case SPEED_LABEL:
> +        {
> +            SpeedLabel* label = new SpeedLabel( p_intf, this );
> +            label->setFrameStyle( QFrame::StyledPanel | QFrame::Raised );
> +            label->setLineWidth( 1 );
> +            widget = label;

Why isn't this done inside the SpeedLabel constructor?

More generally, this seems ok, but you might want to split the time
changes and the speed changes. If you cannot split it, I'll do it.

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device