[vlc-devel] [PATCH 09/10] MED Support - Edit dialogs

[Roiy Shpaner]

Hey, thanks for the review.

> -----Original Message-----
> From: vlc-devel [mailto:vlc-devel-bounces at videolan.org] On Behalf Of
> Francois Cartegnie
> Sent: April-06-14 2:34 AM
> To: Mailing list for VLC media player developers
> Subject: Re: [vlc-devel] [PATCH 09/10] MED Support - Edit dialogs
> 
> 
> > +
> > +private slots:
> > +    friend class    Singleton<DeleteEventDialog>;
> 
> ...

This is the way DialogsProvider calls all the singleton dialogs in the
code...

> 
> > +#ifdef __APPLE__
> > +    setWindowFlags( Qt::Drawer );
> > +#else
> > +    setWindowFlags( Qt::Tool );
> > +#endif
> 
> Dubious.

Removed.

> 
> > +    int64_t startTime = (int64_t) ( QTime( 0, 0, 0 ).msecsTo(
> > + startTimeEdit->time() ) ) * 1000;
> > +
> > +    int64_t endTime = (int64_t) ( QTime( 0, 0, 0 ).msecsTo(
> > + endTimeEdit->time() ) ) * 1000;
> 
> INT64_C
> 
> 
> > +    QDialogButtonBox *editButtonBox = new QDialogButtonBox(
> Qt::Horizontal, this );
> > +    editButtonBox->addButton(
> > +        new QPushButton( qtr("&Edit"), this ),
> > + QDialogButtonBox::AcceptRole );
> > +
> > +    QDialogButtonBox *closeButtonBox = new QDialogButtonBox(
> Qt::Horizontal, this );
> > +    closeButtonBox->addButton(
> > +        new QPushButton( qtr("&Cancel"), this ),
> > + QDialogButtonBox::RejectRole );
> 
> There should be only 1 button box per window.

Ok, fixed.

> 
> > +    if ( mainLayout != NULL )
> > +    {
> > +        QLayoutItem* item;
> > +        while ( ( item = mainLayout->takeAt( 0 ) ) != NULL )
> > +        {
> > +            delete item->widget();
> > +            delete item;
> > +        }
> > +        delete mainLayout;
> > +        mainLayout = NULL;
> > +    }
> 
> See Qt parent-child relationships

Fixed. Only deleting the mainLayout.

> 
> > +    input_Control( p_input, INPUT_SET_TIME, jumpTime );
> > +    vlc_object_release( p_input );
> > +}
> > +
> 
> If you want to access/interact with input, do it through
Main/InputManager.
> Valid for many places.

Ok, I'll change those.

> 
> 
> > +        startTimeIntro->setStyleSheet( "font: bold" );
> font-weight
> 
> > +    QTimeEdit *orgEndTime;
> > +    QLabel *endTimeIntro;
> > +    QTimeEdit *endTimeEdit;
> > +    QPushButton *focusEndButton;
> > +    QPushButton *resetEndButton;
> 
> Most controls likely do not need to be class member.

Most do as their style is changed in different states.  Removed a couple
that don't.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel