Review Request 110080: Fix tooltips of the page navigator of Kexi's report view

Friedrich W. H. Kossebau kossebau at kde.org
Sat Apr 20 00:42:54 BST 2013



> On April 19, 2013, 2:15 p.m., Jarosław Staniek wrote:
> > kexi/widget/utils/kexirecordnavigator.cpp, line 370
> > <http://git.reviewboard.kde.org/r/110080/diff/1/?file=139612#file139612line370>
> >
> >     Can we split to setButtonToolTipText() and setButtonWhatsThis()?
> >     Tooltips tend to be different texts than whatsthis-text.

Results in duplicated code and calls, but as you prefer :)


> On April 19, 2013, 2:15 p.m., Jarosław Staniek wrote:
> > kexi/widget/utils/kexirecordnavigator.cpp, line 374
> > <http://git.reviewboard.kde.org/r/110080/diff/1/?file=139612#file139612line374>
> >
> >     Could we please stay with the switch() as more clear code?

Clear code seems to depend on what people are used to ;)
You are maintainer, so your choice rules :)


> On April 19, 2013, 2:15 p.m., Jarosław Staniek wrote:
> > kexi/widget/utils/kexirecordnavigator.cpp, line 387
> > <http://git.reviewboard.kde.org/r/110080/diff/1/?file=139612#file139612line387>
> >
> >     IMHO It's rare approach to set tooltips for labels (too intrusive). I propose to have setNumberFieldWhatsThis() and setting 'whats this' text only.

Problem is that the current code of KexiRecordNavigator sets tooltips to both the number field and the count field, cmp. constructor code
KexiRecordNavigator::KexiRecordNavigator(QWidget *parent, Q3ScrollView* parentView, int leftMargin)
[...]
    d->navRecordNumber->setToolTip(i18n("Current record number"));
[...]
    d->navRecordCount->setToolTip(i18n("Number of records"));
[...]
}
This is why I added this method at all. Though I now think "setNumberFieldToolTips" is a bad name, should be rather  "setNumberCountFieldsToolTips" or perhaps split into two separate methods, "setNumberFieldToolTip" and "setCountFieldToolTip". Or KexiRecordNavigatorHandler would need to be extended into a model that also provides the action texts :)
But than I would rather work on writing a generic KoListNavigatorWidget, which then can be used in all the places where there currently is a similar tool. But no time for that, I just want to do a quick (n'dirty) fix to the wrong tooltips. :)


- Friedrich W. H.


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


On April 19, 2013, 11:42 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110080/
> -----------------------------------------------------------
> 
> (Updated April 19, 2013, 11:42 p.m.)
> 
> 
> Review request for Calligra and Jarosław Staniek.
> 
> 
> Description
> -------
> 
> Ideally the record/page/... navigator tools of Kexi, Stage, Plan and (?) would be merged into a single class, for consistency and code sharing.
> 
> Until then, let's at least have the page navigator tool in the report view of Kexi show sane tooltips, and not talk about records when there are pages :)
> 
> 
> Diffs
> -----
> 
>   kexi/plugins/reports/kexireportview.cpp cdd0026 
>   kexi/widget/utils/kexirecordnavigator.h b73fe4f 
>   kexi/widget/utils/kexirecordnavigator.cpp b1e2384 
> 
> Diff: http://git.reviewboard.kde.org/r/110080/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130419/c18ff186/attachment.htm>


More information about the calligra-devel mailing list