Discussion:
Bug 338702 and general settings dialog behavior
Tobias Leupold
2014-09-01 19:07:22 UTC
Permalink
Hi list :-)

Yesterday, I found a bug and filed it as Bug #338702. I just uploaded my
proposed patch for that to the new branch bug_338702. Would anybody be so kind
to check if everything is okay with it? I still don't dare to "just" push
changes to master ;-)

Apart from that, I found some other behavior in the category settings dialog
of which I think we should change it (would be no problem for me I think, I
just want to know if I'm right, so that I don't code in vain):

At the moment, it's possible to create a new category with no name by simply
pressing the "New" button and saving the database. Even multiple categories
without a name can be created this way. Further, if we have the standard
"People" category (which is e. g. displayed as "Personen" with a German
locale), one can create a new category with the name "People". Both will be
displayed as "Personen".

I don't think we want to allow all that.

What about leaving everything as-is, but adding some checks if the category
name LineEdit is about to lose it's focus? Like "Please enter a category name"
if it's empty (after stripping out stray whitespace, I think we should always
do that) and something like "You entered the name 'People', which is the un-
localized name for the standard category 'Personen'. This category already
exists, please choose another name".

If something is wrong, the cursor simply stays inside the LineEdit, until a
usable category name is given.

What do you think?

Tobias
Robert Krawitz
2014-09-01 20:30:19 UTC
Permalink
Post by Tobias Leupold
Hi list :-)
Yesterday, I found a bug and filed it as Bug #338702. I just uploaded my proposed patch for that to the new branch bug_338702. Would anybody be so kind to check if everything is okay with it? I still don't dare to "just" push changes to master ;-)
At the moment, it's possible to create a new category with no name by simply pressing the "New" button and saving the database. Even multiple categories without a name can be created this way. Further, if we have the standard "People" category (which is e. g. displayed as "Personen" with a German locale), one can create a new category with the name "People". Both will be displayed as "Personen".
I don't think we want to allow all that.
What about leaving everything as-is, but adding some checks if the category name LineEdit is about to lose it's focus? Like "Please enter a category name" if it's empty (after stripping out stray whitespace, I think we should always do that) and something like "You entered the name 'People', which is the un-
localized name for the standard category 'Personen'. This category already exists, please choose another name".
If something is wrong, the cursor simply stays inside the LineEdit, until a usable category name is given.
What do you think?
I thought I had offered a patch to avoid this problem a while ago;
maybe I forgot to send it in. In any case, your solution sounds more
complete.

--------------------------- Viewer/ViewerWidget.cpp ---------------------------
index 0c024f3..214cb76 100644
@@ -1136,6 +1136,8 @@ void Viewer::ViewerWidget::keyPressEvent( QKeyEvent* event )
_currentInput.left(1) == QString::fromLatin1("\'")) {
_currentInput = _currentInput.right(_currentInput.length()-1);
}
+ if (_currentInput == QString::fromLatin1(""))
+ return;
currentInfo()->addCategoryInfo( DB::ImageDB::instance()->categoryCollection()->nameForText( _currentCategory ), _currentInput );
DB::CategoryPtr category =
DB::ImageDB::instance()->categoryCollection()->categoryForName(_currentCategory);
--
Robert Krawitz <rlk-FrUbXkNCsVf2fBVCVOL8/***@public.gmane.org>

MIT VI-3 1987 - Congrats MIT Engineers 6 straight men's hoops tourney
Tall Clubs International -- http://www.tall.org/ or 1-888-IM-TALL-2
Member of the League for Programming Freedom -- http://ProgFree.org
Project lead for Gutenprint -- http://gimp-print.sourceforge.net

"Linux doesn't dictate how I work, I dictate how Linux works."
--Eric Crampton
Tobias Leupold
2014-09-02 18:36:39 UTC
Permalink
Post by Robert Krawitz
I thought I had offered a patch to avoid this problem a while ago;
maybe I forgot to send it in. In any case, your solution sounds more
complete.
I just uploaded my proposed patch for the empty/duplicate/localized category
name problem to the "category_settings" branch.

This stops the category settings dialog from creating duplicate categories,
categories with no name and categories with the C locale name of an existing
category.

Further, I reworked the layout and did an overall code cleanup.

What do you think?

Apart from that, there's one thing I don't grasp completely at the moment:
what's it with that UntaggedGroupBox class? Why does this exist in an extra
class? The behavior is a bit odd: if one wants to set something there, the
category has to be selected (again) and the category names don't change as
they are edited.

Can't we simply add this functionality to the actual category settings dialog
and remove that class?

Tobias

Loading...