digiKam › digikam-devel

Review Request 108382: Make tagging more accessible by keyboard

classic Classic list List threaded Threaded
♦
♦
 25 messages Options
12
Kusi
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: Review Request 108382: Make tagging more accessible by keyboard

Kusi
This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108382/

On April 26th, 2013, 5:54 p.m. UTC, Marcel Wiesweg wrote:

OK. Sorting is good, the "T" action sounds good.

Putting existing tags to the top and the creation of a new tag to the bottom is a decision based on personal preference. The current behavior favors creating new tags, while the patched behavior favors assigning existing tags by keyboard. Gilles, your opinion?

Some more questions:
- You remove  emit currentCompletionTextChanged(current->data...) Why? Unused, causing a bug?
- Why setCurrentRow(0)?

On April 26th, 2013, 6:30 p.m. UTC, Gilles Caulier wrote:

>Putting existing tags to the top and the creation of a new tag to the bottom is a decision based on personal preference.

So in this case tags list is not shorted by alpha ?

> The current behavior favors creating new tags, 

yes.

>while the patched behavior favors assigning existing tags by keyboard. Gilles, your opinion?

We can considerate this patch introducing a new feature. 

Try to use Tags/New tag menu option from AlbumGUI, it only available when a tag is selected from tag tree-view on the left side. It doesn't work when tag is selected from tree-view on the right side.

I we want to be homogeneous, It will be judicious to add a keyboard shortcut to this action, and whole context management must be changed to enable it when focus is given to one of all tags tree-view... No ?

Gilles

Marcel, Gilles,  sorry for the late response! 

>> You remove  emit currentCompletionTextChanged(current->data...) Why?
assume your list of most recently used tags only consists of "Paris". Now, if you press "P" in the TagEdit-box "emit currentCompletionTextChanged(current->data...)" will be fired, where the parameter passed to "currentCompletionTextChanged" already contains "Paris". This means the TagEdit-box immediately contains "Paris" after pressing "P". There is no way for the user to add a new tag "Peking" via TagEdit-box. If I start deleting the last letter "Pari", "currentCompletionTextChanged" would be fired again and TagsCompletionBox again contains "Paris".

>> Why setCurrentRow(0)?
After filling up all the items in the setItems method, no row is selected in the TagsCompletionBox. Since the items are already correctly ordered, the most likely candidate will ALWAYS be on the first row, hence parameter 0 for setCurrentRow(). If no item was selected (no setCurrentRow call), the current (first) row would not be returned, instead the so far entered text would be used. Example: your list contains of "Paris", you press "Pa" & enter. Without setCurrentRow(0), a new tag "Pa" would be created.

>> The current behavior favors creating new tags, while the patched behavior favors assigning existing tags by keyboard
This is not entirely true: if you create a new tag, you need to write the entire tag, anyways. As soon as the new tagname does not match anymore with recently assigned tags, you'll have the same choice at the first two rows of the the CompletionBox as it is now. There is one exception: you have "Parisienne" in your list and you want to create the tag "Paris". Then indeed the existing tag is favored.

>> Try to use Tags/New tag menu option from AlbumGUI, it only available when a tag is selected from tag tree-view on the 
>> left side. It doesn't work when tag is selected from tree-view on the right side.
Yes, indeed. I never used Tags/New. In my opinion, the entire "Tags" menu entry is probably not necessary, there are already several other options to browse,add,delete tags. 

best regards, Markus

- Markus


On April 21st, 2013, 1:04 p.m. UTC, Markus Leuthold wrote:

Review request for Digikam, Gilles Caulier and Marcel Wiesweg.
By Markus Leuthold.

Updated April 21, 2013, 1:04 p.m.

Description 

Make tagging more accessible by keyboard

 * Pressing "T" will focus the tagedit-box.
 * The tag is applied by pressing enter. Pressing enter a second time will
   focus the mainwindow and advance to the next image.
 * The dropdown of the tagedit-box remembers already entered tags, the
   order of the dropdown items are sorted such that already entered tags appear first.
 * add tagging-by-keyboard to "Tips of the day"

Testing 

I successfully use this feature on a regular basis. Also tested with current HEAD, works fine.

Diffs

  • data/tips (eef2262ef39fe77a65113f75be750122bda0f3fb)
  • digikam/main/digikamapp.cpp (d7769e60d4181f95149774744fb93e711761c2ed)
  • digikam/main/digikamapp_p.h (2d9f0c20f65a29db153c868cbfcea2e5574b1bdd)
  • digikam/tags/addtagscompletionbox.cpp (266421c34fe6d939b094d0c0b1dea77065e024ee)
  • digikam/tags/addtagslineedit.h (1eec38c090eaefd592e4cc5c4561fadf04b9de26)
  • digikam/tags/addtagslineedit.cpp (abcf5b1f9d9a0340a9838b267faa2637f989bd96)
  • digikam/views/digikamview.h (669eac958eeaa5aacb52f0a55d879175c4abbe34)
  • digikam/views/digikamview.cpp (89ae95684cef96da14df1807ab821f04d274471a)
  • libs/database/albumdb.cpp (8ca4fd0e0323fb7c8bae65f947d21b9d9ee6b50c)
  • libs/imageproperties/imagedescedittab.h (0dc7e31af05bf95a6caf5ee4edde962bda7c0e7a)
  • libs/imageproperties/imagedescedittab.cpp (b6ce9494852b4a624addefa7c1fb1196ff265e68)
  • libs/imageproperties/imagepropertiessidebar.h (f6703339fc2e1a5762f8db898e7ad69dddab7868)

View Diff

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Kusi
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: Review Request 108382: Make tagging more accessible by keyboard

Kusi
In reply to this post by Marcel Wiesweg
This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108382/

On April 26th, 2013, 5:54 p.m. UTC, Marcel Wiesweg wrote:

OK. Sorting is good, the "T" action sounds good.

Putting existing tags to the top and the creation of a new tag to the bottom is a decision based on personal preference. The current behavior favors creating new tags, while the patched behavior favors assigning existing tags by keyboard. Gilles, your opinion?

Some more questions:
- You remove  emit currentCompletionTextChanged(current->data...) Why? Unused, causing a bug?
- Why setCurrentRow(0)?

On April 26th, 2013, 6:30 p.m. UTC, Gilles Caulier wrote:

>Putting existing tags to the top and the creation of a new tag to the bottom is a decision based on personal preference.

So in this case tags list is not shorted by alpha ?

> The current behavior favors creating new tags, 

yes.

>while the patched behavior favors assigning existing tags by keyboard. Gilles, your opinion?

We can considerate this patch introducing a new feature. 

Try to use Tags/New tag menu option from AlbumGUI, it only available when a tag is selected from tag tree-view on the left side. It doesn't work when tag is selected from tree-view on the right side.

I we want to be homogeneous, It will be judicious to add a keyboard shortcut to this action, and whole context management must be changed to enable it when focus is given to one of all tags tree-view... No ?

Gilles

On May 30th, 2013, 10:03 p.m. UTC, Markus Leuthold wrote:

Marcel, Gilles,  sorry for the late response! 

>> You remove  emit currentCompletionTextChanged(current->data...) Why?
assume your list of most recently used tags only consists of "Paris". Now, if you press "P" in the TagEdit-box "emit currentCompletionTextChanged(current->data...)" will be fired, where the parameter passed to "currentCompletionTextChanged" already contains "Paris". This means the TagEdit-box immediately contains "Paris" after pressing "P". There is no way for the user to add a new tag "Peking" via TagEdit-box. If I start deleting the last letter "Pari", "currentCompletionTextChanged" would be fired again and TagsCompletionBox again contains "Paris".

>> Why setCurrentRow(0)?
After filling up all the items in the setItems method, no row is selected in the TagsCompletionBox. Since the items are already correctly ordered, the most likely candidate will ALWAYS be on the first row, hence parameter 0 for setCurrentRow(). If no item was selected (no setCurrentRow call), the current (first) row would not be returned, instead the so far entered text would be used. Example: your list contains of "Paris", you press "Pa" & enter. Without setCurrentRow(0), a new tag "Pa" would be created.

>> The current behavior favors creating new tags, while the patched behavior favors assigning existing tags by keyboard
This is not entirely true: if you create a new tag, you need to write the entire tag, anyways. As soon as the new tagname does not match anymore with recently assigned tags, you'll have the same choice at the first two rows of the the CompletionBox as it is now. There is one exception: you have "Parisienne" in your list and you want to create the tag "Paris". Then indeed the existing tag is favored.

>> Try to use Tags/New tag menu option from AlbumGUI, it only available when a tag is selected from tag tree-view on the 
>> left side. It doesn't work when tag is selected from tree-view on the right side.
Yes, indeed. I never used Tags/New. In my opinion, the entire "Tags" menu entry is probably not necessary, there are already several other options to browse,add,delete tags. 

best regards, Markus





The proposed patch needs some minor modifications, due to rebasing on current master. I successfully use this patch now already for quite a while, may I commit? 

best regards
Markus

- Markus


On April 21st, 2013, 1:04 p.m. UTC, Markus Leuthold wrote:

Review request for Digikam, Gilles Caulier and Marcel Wiesweg.
By Markus Leuthold.

Updated April 21, 2013, 1:04 p.m.

Description 

Make tagging more accessible by keyboard

 * Pressing "T" will focus the tagedit-box.
 * The tag is applied by pressing enter. Pressing enter a second time will
   focus the mainwindow and advance to the next image.
 * The dropdown of the tagedit-box remembers already entered tags, the
   order of the dropdown items are sorted such that already entered tags appear first.
 * add tagging-by-keyboard to "Tips of the day"

Testing 

I successfully use this feature on a regular basis. Also tested with current HEAD, works fine.

Diffs

  • data/tips (eef2262ef39fe77a65113f75be750122bda0f3fb)
  • digikam/main/digikamapp.cpp (d7769e60d4181f95149774744fb93e711761c2ed)
  • digikam/main/digikamapp_p.h (2d9f0c20f65a29db153c868cbfcea2e5574b1bdd)
  • digikam/tags/addtagscompletionbox.cpp (266421c34fe6d939b094d0c0b1dea77065e024ee)
  • digikam/tags/addtagslineedit.h (1eec38c090eaefd592e4cc5c4561fadf04b9de26)
  • digikam/tags/addtagslineedit.cpp (abcf5b1f9d9a0340a9838b267faa2637f989bd96)
  • digikam/views/digikamview.h (669eac958eeaa5aacb52f0a55d879175c4abbe34)
  • digikam/views/digikamview.cpp (89ae95684cef96da14df1807ab821f04d274471a)
  • libs/database/albumdb.cpp (8ca4fd0e0323fb7c8bae65f947d21b9d9ee6b50c)
  • libs/imageproperties/imagedescedittab.h (0dc7e31af05bf95a6caf5ee4edde962bda7c0e7a)
  • libs/imageproperties/imagedescedittab.cpp (b6ce9494852b4a624addefa7c1fb1196ff265e68)
  • libs/imageproperties/imagepropertiessidebar.h (f6703339fc2e1a5762f8db898e7ad69dddab7868)

View Diff

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Marcel Wiesweg
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: Review Request 108382: Make tagging more accessible by keyboard

Marcel Wiesweg
In reply to this post by Kusi
This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108382/

Ship it!

Ship It!

- Marcel


On April 21st, 2013, 1:04 p.m. UTC, Markus Leuthold wrote:

Review request for Digikam, Gilles Caulier and Marcel Wiesweg.
By Markus Leuthold.

Updated April 21, 2013, 1:04 p.m.

Description 

Make tagging more accessible by keyboard

 * Pressing "T" will focus the tagedit-box.
 * The tag is applied by pressing enter. Pressing enter a second time will
   focus the mainwindow and advance to the next image.
 * The dropdown of the tagedit-box remembers already entered tags, the
   order of the dropdown items are sorted such that already entered tags appear first.
 * add tagging-by-keyboard to "Tips of the day"

Testing 

I successfully use this feature on a regular basis. Also tested with current HEAD, works fine.

Diffs

  • data/tips (eef2262ef39fe77a65113f75be750122bda0f3fb)
  • digikam/main/digikamapp.cpp (d7769e60d4181f95149774744fb93e711761c2ed)
  • digikam/main/digikamapp_p.h (2d9f0c20f65a29db153c868cbfcea2e5574b1bdd)
  • digikam/tags/addtagscompletionbox.cpp (266421c34fe6d939b094d0c0b1dea77065e024ee)
  • digikam/tags/addtagslineedit.h (1eec38c090eaefd592e4cc5c4561fadf04b9de26)
  • digikam/tags/addtagslineedit.cpp (abcf5b1f9d9a0340a9838b267faa2637f989bd96)
  • digikam/views/digikamview.h (669eac958eeaa5aacb52f0a55d879175c4abbe34)
  • digikam/views/digikamview.cpp (89ae95684cef96da14df1807ab821f04d274471a)
  • libs/database/albumdb.cpp (8ca4fd0e0323fb7c8bae65f947d21b9d9ee6b50c)
  • libs/imageproperties/imagedescedittab.h (0dc7e31af05bf95a6caf5ee4edde962bda7c0e7a)
  • libs/imageproperties/imagedescedittab.cpp (b6ce9494852b4a624addefa7c1fb1196ff265e68)
  • libs/imageproperties/imagepropertiessidebar.h (f6703339fc2e1a5762f8db898e7ad69dddab7868)

View Diff

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Gilles Caulier-5
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: Review Request 108382: Make tagging more accessible by keyboard

Gilles Caulier-5
In reply to this post by Kusi
This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108382/ 

This review has been submitted with commit d13f94a8c5cf18a7c3c19c522fa484e5b7fb1bd9 by Markus Leuthold to branch master.

- Commit


On April 21st, 2013, 1:04 p.m. UTC, Markus Leuthold wrote:

Review request for Digikam, Gilles Caulier and Marcel Wiesweg.
By Markus Leuthold.

Updated April 21, 2013, 1:04 p.m.

Description 

Make tagging more accessible by keyboard

 * Pressing "T" will focus the tagedit-box.
 * The tag is applied by pressing enter. Pressing enter a second time will
   focus the mainwindow and advance to the next image.
 * The dropdown of the tagedit-box remembers already entered tags, the
   order of the dropdown items are sorted such that already entered tags appear first.
 * add tagging-by-keyboard to "Tips of the day"

Testing 

I successfully use this feature on a regular basis. Also tested with current HEAD, works fine.

Diffs

  • data/tips (eef2262ef39fe77a65113f75be750122bda0f3fb)
  • digikam/main/digikamapp.cpp (d7769e60d4181f95149774744fb93e711761c2ed)
  • digikam/main/digikamapp_p.h (2d9f0c20f65a29db153c868cbfcea2e5574b1bdd)
  • digikam/tags/addtagscompletionbox.cpp (266421c34fe6d939b094d0c0b1dea77065e024ee)
  • digikam/tags/addtagslineedit.h (1eec38c090eaefd592e4cc5c4561fadf04b9de26)
  • digikam/tags/addtagslineedit.cpp (abcf5b1f9d9a0340a9838b267faa2637f989bd96)
  • digikam/views/digikamview.h (669eac958eeaa5aacb52f0a55d879175c4abbe34)
  • digikam/views/digikamview.cpp (89ae95684cef96da14df1807ab821f04d274471a)
  • libs/database/albumdb.cpp (8ca4fd0e0323fb7c8bae65f947d21b9d9ee6b50c)
  • libs/imageproperties/imagedescedittab.h (0dc7e31af05bf95a6caf5ee4edde962bda7c0e7a)
  • libs/imageproperties/imagedescedittab.cpp (b6ce9494852b4a624addefa7c1fb1196ff265e68)
  • libs/imageproperties/imagepropertiessidebar.h (f6703339fc2e1a5762f8db898e7ad69dddab7868)

View Diff

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Gilles Caulier-5
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: Review Request 108382: Make tagging more accessible by keyboard

Gilles Caulier-5
In reply to this post by Kusi
This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108382/

This change has been marked as submitted.

Review request for Digikam, Gilles Caulier and Marcel Wiesweg.
By Markus Leuthold.

Updated Sept. 9, 2013, 9:39 p.m.

Description 

Make tagging more accessible by keyboard

 * Pressing "T" will focus the tagedit-box.
 * The tag is applied by pressing enter. Pressing enter a second time will
   focus the mainwindow and advance to the next image.
 * The dropdown of the tagedit-box remembers already entered tags, the
   order of the dropdown items are sorted such that already entered tags appear first.
 * add tagging-by-keyboard to "Tips of the day"

Testing 

I successfully use this feature on a regular basis. Also tested with current HEAD, works fine.

Diffs

  • data/tips (eef2262ef39fe77a65113f75be750122bda0f3fb)
  • digikam/main/digikamapp.cpp (d7769e60d4181f95149774744fb93e711761c2ed)
  • digikam/main/digikamapp_p.h (2d9f0c20f65a29db153c868cbfcea2e5574b1bdd)
  • digikam/tags/addtagscompletionbox.cpp (266421c34fe6d939b094d0c0b1dea77065e024ee)
  • digikam/tags/addtagslineedit.h (1eec38c090eaefd592e4cc5c4561fadf04b9de26)
  • digikam/tags/addtagslineedit.cpp (abcf5b1f9d9a0340a9838b267faa2637f989bd96)
  • digikam/views/digikamview.h (669eac958eeaa5aacb52f0a55d879175c4abbe34)
  • digikam/views/digikamview.cpp (89ae95684cef96da14df1807ab821f04d274471a)
  • libs/database/albumdb.cpp (8ca4fd0e0323fb7c8bae65f947d21b9d9ee6b50c)
  • libs/imageproperties/imagedescedittab.h (0dc7e31af05bf95a6caf5ee4edde962bda7c0e7a)
  • libs/imageproperties/imagedescedittab.cpp (b6ce9494852b4a624addefa7c1fb1196ff265e68)
  • libs/imageproperties/imagepropertiessidebar.h (f6703339fc2e1a5762f8db898e7ad69dddab7868)

View Diff

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
12
Free forum by Nabble Edit this page