D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R824:c36d6b26eb3f: [FileMetaDataWidget] Allow inline 
configuration of visible properties (authored by bruns).

REPOSITORY
  R824 Baloo Widgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20524?vs=56545=56725

REVISION DETAIL
  https://phabricator.kde.org/D20524

AFFECTED FILES
  src/filemetadatawidget.cpp
  src/filemetadatawidget.h

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich, elvisangelaccio
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Elvis Angelaccio]

elvisangelaccio accepted this revision.

REPOSITORY
  R824 Baloo Widgets

BRANCH
  metadata_config_inline

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich, elvisangelaccio
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Nathaniel Graham]

ngraham accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R824 Baloo Widgets

BRANCH
  metadata_config_inline

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns updated this revision to Diff 56545.
bruns added a comment.


  reset column stretch

REPOSITORY
  R824 Baloo Widgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20524?vs=56536=56545

BRANCH
  metadata_config_inline

REVISION DETAIL
  https://phabricator.kde.org/D20524

AFFECTED FILES
  src/filemetadatawidget.cpp
  src/filemetadatawidget.h

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns added a dependent revision: D20667: Make the FileMetadataWidget vertical 
stretch in a sensible way.

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns updated this revision to Diff 56536.
bruns added a comment.


  Layout cleanup
  
  - remove extra spacer
  - keep checkbox near label (align right)

REPOSITORY
  R824 Baloo Widgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20524?vs=56337=56536

BRANCH
  metadata_config_inline

REVISION DETAIL
  https://phabricator.kde.org/D20524

AFFECTED FILES
  src/filemetadatawidget.cpp
  src/filemetadatawidget.h

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns marked an inline comment as done.

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns updated this revision to Diff 56337.
bruns added a comment.


  correct since version

REPOSITORY
  R824 Baloo Widgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20524?vs=56206=56337

BRANCH
  metadata_config_inline

REVISION DETAIL
  https://phabricator.kde.org/D20524

AFFECTED FILES
  src/filemetadatawidget.cpp
  src/filemetadatawidget.h

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> filemetadatawidget.h:93
> + * Switch between regular (view) and configuration mode.
> + * @since 5.58
> + */

Should be `@since 19.08.0` (baloo-widgets follows the KDE Applications 
versioning)

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns marked 4 inline comments as done.

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns updated this revision to Diff 56206.
bruns added a comment.


  add apidox, cleanup

REPOSITORY
  R824 Baloo Widgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20524?vs=56166=56206

BRANCH
  metadata_config_inline

REVISION DETAIL
  https://phabricator.kde.org/D20524

AFFECTED FILES
  src/filemetadatawidget.cpp
  src/filemetadatawidget.h

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns added a comment.


  In D20524#449784 , @ngraham wrote:
  
  > In D20524#449668 , 
@elvisangelaccio wrote:
  >
  > > Hmm, it seems you need to rebase this patch:
  >
  >
  > Worked for me:
  >
  >INFO  Base commit is not in local repository; trying to fetch.
  >   Created and checked out branch arcpatch-D20524.
  >  
  >  
  >   This diff is against commit b26a3680b04cb9877721d693eb79ccb321fe40aa, 
but
  >   the commit is nowhere in the working copy. Try to apply it against the
  >   current working copy state? (5f2ab5c62dd56e5a1dae468355c9e72d84d94398)
  >   [Y/n] y
  >  
  >   Checking patch src/filemetadatawidget.h...
  >   Checking patch src/filemetadatawidget.cpp...
  >   Applied patch src/filemetadatawidget.h cleanly.
  >   Applied patch src/filemetadatawidget.cpp cleanly.
  >OKAY  Successfully committed patch.
  >
  
  
  I have some debug output lower in the commit stack, which is likely 
responsible for the SHA hash mismatch, although it should apply with just the 
warning.
  
  Note, D20523  was commited in the mean 
time.

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Nathaniel Graham]

ngraham added a comment.


  In D20524#449668 , 
@elvisangelaccio wrote:
  
  > Hmm, it seems you need to rebase this patch:
  
  
  Worked for me:
  
 INFO  Base commit is not in local repository; trying to fetch.
Created and checked out branch arcpatch-D20524.


This diff is against commit b26a3680b04cb9877721d693eb79ccb321fe40aa, 
but
the commit is nowhere in the working copy. Try to apply it against the
current working copy state? (5f2ab5c62dd56e5a1dae468355c9e72d84d94398)
[Y/n] y

Checking patch src/filemetadatawidget.h...
Checking patch src/filemetadatawidget.cpp...
Applied patch src/filemetadatawidget.h cleanly.
Applied patch src/filemetadatawidget.cpp cleanly.
 OKAY  Successfully committed patch.

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns edited the summary of this revision.

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in filemetadatawidget.h:47-49
> Can you add some apidox for this enum? In particular, it's not so clear what 
> `ReStart` is used for.

Sure, just wanted to see if the (UI) design itself is deemed appropriate.

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Elvis Angelaccio]

elvisangelaccio added a comment.


  Hmm, it seems you need to rebase this patch:
  
$ arc patch D20524
 INFO  Base commit is not in local repository; trying to fetch.
Created and checked out branch arcpatch-D20524.


This diff is against commit b26a3680b04cb9877721d693eb79ccb321fe40aa, 
but
the commit is nowhere in the working copy. Try to apply it against the
current working copy state? (49e778edf7bd70842879cc338fb85c1a178ecf7f)
[Y/n] Y
  
  ( I tried to apply it on current master)

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> filemetadatawidget.cpp:53
>  {
> +QCheckBox* check;
>  QLabel* label;

I'd call it `checkBox` for better clarity.

> filemetadatawidget.cpp:113-114
>  row.value->deleteLater();
> +if (row.check)
> +row.check->deleteLater();
>  }

Missing braces

> filemetadatawidget.h:47-49
> +ReStart = 0,
> +Accept,
> +Cancel

Can you add some apidox for this enum? In particular, it's not so clear what 
`ReStart` is used for.

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: elvisangelaccio, gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns added a dependent revision: D20525: [InformationPanel] Use the new inline 
configuration mode.

REPOSITORY
  R824 Baloo Widgets

REVISION DETAIL
  https://phabricator.kde.org/D20524

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: gennad, domson, ashaposhnikov, astippich, spoorun, abrahams

D20524: [FileMetaDataWidget] Allow inline configuration of visible properties

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, Dolphin, Frameworks, ngraham, astippich.
Herald added a project: Baloo.
bruns requested review of this revision.

REVISION SUMMARY
  Instead of opening an additional dialog, allow selection and deselection
  of visible properties directly in the widget.

REPOSITORY
  R824 Baloo Widgets

BRANCH
  metadata_config_inline

REVISION DETAIL
  https://phabricator.kde.org/D20524

AFFECTED FILES
  src/filemetadatawidget.cpp
  src/filemetadatawidget.h

To: bruns, #baloo, #dolphin, #frameworks, ngraham, astippich
Cc: gennad, domson, ashaposhnikov, astippich, spoorun, abrahams