Review Request 108382: Make tagging more accessible by keyboard
Locked
12
12
 
|
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
Thanks Marcus. I set this patch to review before 3.0.0 release... Gilles Caulier - Gilles On January 13th, 2013, 10:16 a.m. UTC, Markus Leuthold wrote:
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
|
|
On Sunday 13 January 2013 10.24:51 Gilles Caulier wrote:
> > This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108382/ > > > Thanks Marcus. I set this patch to review before 3.0.0 release... > > Gilles Caulier > > - Gilles Hello Gilles Thanks for reviewing! I have commit rights, but didn't want to commit it without review, since it is my first commit to the main digikam application (I already did a kipi-plugin, though). Is this the correct workflow? Do I need to put my changes on the review board or can I just commit? best regards, Markus _______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
well, typically, we prefer to use bugzilla to manage patches. We use rarely reviewboard Personalty, i think review board is a duplicate of bugzilla. With bugzilla, we can do all reviewboard feature plus more, as version management, etc... Why 2 management project tools... It more complex workflow.
But it's just my viewpoint... 2013/1/13 Markus Leuthold <[hidden email]>
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
In reply to this post by Kusi
patch not yet tested, but i have a question : You assign T to focus tagsedit view. DO you now that we have already a Tags keyboard shortcuts manager ? What's happen is someone as already created a tag shortcut with 'T' ? - Gilles On January 13th, 2013, 10:16 a.m. UTC, Markus Leuthold wrote:
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
In reply to this post by Kusi
I take a look to your patch (not yet tested) Your new d->assignTagAction is not visible to Tags main menu. It's normal ? It just a cached keyboard shortcut ? Do you see that there is 2 way to manage tags assignment from GUI: 1/ Tags album view from left sidebar 2/ Tags tree view from right sidebar If i'm not too wrong you only manage 2/, to force focus on it. Right ? - Gilles On January 13th, 2013, 10:16 a.m. UTC, Markus Leuthold wrote:
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
|
|
In reply to this post by Gilles Caulier-4
On Sunday 13 January 2013 11.24:38 Gilles Caulier wrote:
> This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108382/ > > > patch not yet tested, but i have a question : You assign T to focus tagsedit view. DO you now that we have already a Tags keyboard shortcuts manager ? What do you mean with "Tags keyboard shortcuts manager"? You mean the possibility to assign a shortcut to a tag? This is only a good thing if you need to assign always the same few tags, since you need to manually assign a shortcut for each tag yourself. With my patch, you implicitly have something like shortcuts for EACH tag. (like vi-editor like tagging) Assume you have two photos, the first photo needs to be tagged with "A" and "B", the second with "C". What you can do with my patch (focus on first photo) Press T, A, Enter,B, Enter, Enter, C, Enter > What's happen is someone as already created a tag shortcut with 'T' ? > You cannot assign "T" to a shortcut, you need at least one modifier (why does digikam decide to have this restriction?). Markus _______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
|
|
In reply to this post by Gilles Caulier-4
On Sunday 13 January 2013 11.55:12 Gilles Caulier wrote:
> This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108382/ > > > I take a look to your patch (not yet tested) > > Your new d->assignTagAction is not visible to Tags main menu. It's normal ? It just a cached keyboard shortcut ? It is a keyboard-only function, no need to expose it to the Tags main menu. If you want to use the mouse, you're there in two clicks > > Do you see that there is 2 way to manage tags assignment from GUI: > 1/ Tags album view from left sidebar > 2/ Tags tree view from right sidebar > > If i'm not too wrong you only manage 2/, to force focus on it. Right ? yes, I only care about the right sidebar (assignment). No need to add a shortcut for the left sidebar(filtering), in my opinion. Markus _______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
2013/1/13 Markus Leuthold <[hidden email]>
Left side is not filtering, it's the virtual tab album view. Tags Filtering in on the right side, in Filters tab.
Gilles _______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
In reply to this post by Kusi
The only changes I dont understand without explanation are those to AddTagsCompletionBox::setItems. What is achieved here? - Marcel On January 13th, 2013, 10:16 a.m. UTC, Markus Leuthold wrote:
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
In reply to this post by Gilles Caulier-4
On Sun, Jan 13, 2013 at 12:21 PM, Gilles Caulier <[hidden email]> wrote:
The main advantage of ReviewBoard over Bugzilla is the ability to comment on code directly and cleanly, for example look at this review request - https://git.reviewboard.kde.org/r/103117/ - scroll down a bit and there's gray bar saying "Dario Freddi" - expand it and you will see Dario's comments plus my replies, all very clear and traceable. It also supports patch revisions so you can see only the changes between two diffs if you want. All in all, it's more comfortable way of reviewing patches. But of course, use whatever suits you best :)
Cheers Martin Klapetek | KDE Developer
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
Hi Martin, I will be more pragmatic : why review board is a separated tool than bugzilla. It must be merged to bugzilla... We work exclusively with bugzilla to try to do the best about AQ. Add another way to fix problems will increase complexity of developer workflow and history. It just my experience of large projects that i manage in my office.
Gilles 2013/1/13 Martin Klapetek <[hidden email]>
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
Hi,
On Mon, Jan 14, 2013 at 9:52 AM, Gilles Caulier <[hidden email]> wrote: --
Well bugzilla is for managing bugs, reviewboard is for reviewing patches (and it's defacto standard around KDE projects). Bugzilla has different usecases and objectives so I don't think we will see any merge in the near future :)
Of course, each project has its own workflow and I most certainly do not intend to change the one DigiKam has, just pointing out why reviewboard is useful as a tool for developers (we push every non-one-line patch in KDE Telepathy through it).
Cheers Martin Klapetek | KDE Developer
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
|
|
In reply to this post by Marcel Wiesweg
Assume you have three tags "Albert", "Alice" and "Bertha" (parent tag is "People"). You want to assign "Alice" to your photo. With current digikam, you get the following situation after each keystroke: keystroke:items displayed "A":"Create A in People", "Albert","Alice","Create A" (Case A as labeled in sourcecode) "l":"Create Al in People", "Albert","Alice","Create Al" (Case A) "i":"Create Ali in People","Alice","Create Ali" (Case A) "c":"Create Alic in People","Alice","Create Alic" (Case A) "e":"Alice","Create Alice" (Case C) Please note that only after the fifth character, you can press enter to commit your tag. The first item is always selected, you can only press enter if your desired item is at position one. Now let's look at my patch, same scenario "A":"Albert","Alice","Create A in People","Create A" (Case A) "l":"Albert","Alice","Create Al in People","Create Al" (Case A) "i":"Alice","Create Ali in People","Create Ali" (Case A) After the 3rd character, you can press enter. Since I remember the chosen tags, "Alice" will be the first item for another photo (this logic is not in the setItems function, though). Assume you want to tag a second photo with "Alice" "A":"Alice" "Albert" "Create A in People" "Create A" Now you can already press enter after the first character, and you're done. With the current digikam version, you need to write the entire name. What I have to admit: I don't know how you can actually get into case B (no item selected at all). I checked with debug statements, and didn't figure out a way to hit that codepath, with current digikam as well as with my patch applied. But if this codepath is hit, the code should behave correctly. Does it become more clear to you what my modifications in setItems do? - Markus On January 13th, 2013, 10:16 a.m. UTC, Markus Leuthold wrote:
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
|
|
may I commit?
best regards, Markus
On Tuesday 15 January 2013 22.27:22 Markus Leuthold wrote:
On January 13th, 2013, 6:20 p.m. UTC, Marcel Wiesweg wrote: The only changes I dont understand without explanation are those to AddTagsCompletionBox::setItems. What is achieved here? Assume you have three tags "Albert", "Alice" and "Bertha" (parent tag is "People"). You want to assign "Alice" to your photo. With current digikam, you get the following situation after each keystroke:
keystroke:items displayed "A":"Create A in People", "Albert","Alice","Create A" (Case A as labeled in sourcecode) "l":"Create Al in People", "Albert","Alice","Create Al" (Case A) "i":"Create Ali in People","Alice","Create Ali" (Case A) "c":"Create Alic in People","Alice","Create Alic" (Case A) "e":"Alice","Create Alice" (Case C)
Please note that only after the fifth character, you can press enter to commit your tag. The first item is always selected, you can only press enter if your desired item is at position one. Now let's look at my patch, same scenario
"A":"Albert","Alice","Create A in People","Create A" (Case A) "l":"Albert","Alice","Create Al in People","Create Al" (Case A) "i":"Alice","Create Ali in People","Create Ali" (Case A)
After the 3rd character, you can press enter. Since I remember the chosen tags, "Alice" will be the first item for another photo (this logic is not in the setItems function, though). Assume you want to tag a second photo with "Alice"
"A":"Alice" "Albert" "Create A in People" "Create A"
Now you can already press enter after the first character, and you're done. With the current digikam version, you need to write the entire name.
What I have to admit: I don't know how you can actually get into case B (no item selected at all). I checked with debug statements, and didn't figure out a way to hit that codepath, with current digikam as well as with my patch applied. But if this codepath is hit, the code should behave correctly.
Does it become more clear to you what my modifications in setItems do?
- Markus On January 13th, 2013, 10:16 a.m. UTC, Markus Leuthold wrote:
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
|
|
In reply to this post by Kusi
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
In reply to this post by Marcel Wiesweg
Marcel, What's about this entry. Previous explanation is fine for you ? Did you test the patch to check feature ? Gilles - Gilles On April 21st, 2013, 1:04 p.m. UTC, Markus Leuthold wrote:
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
In reply to this post by Kusi
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)? - Marcel On April 21st, 2013, 1:04 p.m. UTC, Markus Leuthold wrote:
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
>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 - Gilles On April 21st, 2013, 1:04 p.m. UTC, Markus Leuthold wrote:
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
Re: Review Request 108382: Make tagging more accessible by keyboard
|
|
Hi, So what should be done regarding this patch should it be applied to digiKam git repo.Also since patch works and does not break anything what else has to be done in this bug.On Sat, Apr 27, 2013 at 12:00 AM, Gilles Caulier <[hidden email]> wrote:
_______________________________________________ Digikam-devel mailing list [hidden email] https://mail.kde.org/mailman/listinfo/digikam-devel |
«
Return to digikam-devel
|
1 view|%1 views
| Free forum by Nabble | Edit this page |