Review Request: Fix problem with inline editing in playlist.

Ralf Engels ralf-engels at gmx.de
Mon Oct 18 20:33:06 CEST 2010



> On 2010-10-15 07:46:32, Mark Kretschmann wrote:
> > Ralf, with my "Verbose" playlist layout, all the lines have become about 50% taller with your patch. So the playlist takes up much more vertical space.
> > 
> > Is this intended? I'm not sure it's a good idea to use so much vertical space.
> >
> 
> Ralf Engels wrote:
>     The reason for the fix was that when editing an entry in the collection view the space was not enough to display the line edits and rating widget completely.
>     This starts with a two lined layout where the second line edit is missing the two lowest pixels.
>     It get's worse when editing a compressed (as in two songs from the same album) entry where the line edits overlap the header.
>     It is completely unusable if you have a third line with only a rating widget.
>     
>     Only three solutions come to my mind:
>     1. allow the edit components to overlap.
>     This is not really nice as we would overlap uncontrolled e.g. at the end of the list.
>     2. change the layout of the item when going into edit mode.
>     I don't really know how to do this nicely.
>     3. Calculate the height of the items so that the edit widgets will have their minimum size.
>     That is what I am doing. 
>     I am even cheating a little because in the "normal" mode I allow a margin around the item which is not there in the edit mode.
>     Also now the list has the same layout as all the other lists, which you can see when comparing the item height against the item height of e.g. the dynamic playlist items.
>     
>     I would rather have a little bit larger list that supports three row layouts.
> 
> Nikolaj Hald Nielsen wrote:
>     How about simply changing the style of the edit boxes to make them the same height as a line of text? You can do some pretty cool stuff with stylesheets.
>     
>     I dont think that making each line in the playlist taller for all users in _all_ cases, even the ones that never use the inline editing is acceptable.
> 
> Thomas Lübking wrote:
>     w/o having looked at the code you likely want
>     QLineEdit::setFrame(false);
>     this should shrink the size and if everything else fails you could still
>     QWidget::setFixedHeight(h);
>     
>     So there's likely no need to hardcode stylesheets - which can easily cause trouble by hardcoding colors :-(
>     
>     Otherwise i'm with Nikolaj - styles could define *large* paddings for lineedits (eg. for a "match button height" policy or so) and i may recall that there've been various mourns that "the new $%&/()= playlist wastes too much space" =P
>     
>     
>     ... ok and actually looking at the code:
>     the proper function for styled lineedit heights is "QStyle::sizeFromContents(CT_LineEdit, .*)" and not some pixelmetrics + whatever oxygen randomly adds to it, thanks alot ;-)

Good comments.
1. Even with a stylesheet you cannot go below the widget minimum size.
2. The styles really define a lot of paddings and frame sizes. Oxygen e.g. has a 11 pixel frame as far as I remember. To compensate for it it has a -1 px padding. The old code had some hard coded workarounds for this.
3. good idea with setFrame. That could be the solution. I will try that out.
4. sizeFromContents is only half the solution. QLineEdit adds a couple of stuff to it (QLineEdit.cpp:646)
 But next time I try to use that.


- Ralf


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


On 2010-10-09 18:27:00, Ralf Engels wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100039/
> -----------------------------------------------------------
> 
> (Updated 2010-10-09 18:27:00)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> -------
> 
> Fixes three problems:
> 1. Alignment was not considered with rating widget when editing
> 2. Calculation of item height was incorrect leading to the problem that three line items would not show the third line when editing
> 3. QStyle pixel metrics were disregarded
> 
> 
> Diffs
> -----
> 
>   src/playlist/view/listview/InlineEditorWidget.h f14d6df 
>   src/playlist/view/listview/InlineEditorWidget.cpp a7e981b 
>   src/playlist/view/listview/PrettyItemDelegate.h a929ecb 
>   src/playlist/view/listview/PrettyItemDelegate.cpp 31d4379 
>   src/playlist/view/listview/PrettyListView.cpp ca3d580 
> 
> Diff: http://git.reviewboard.kde.org/r/100039/diff
> 
> 
> Testing
> -------
> 
> Editing with one, two and three line layouts.
> Using Oxygen and normal styles.
> 
> 
> Thanks,
> 
> Ralf
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20101018/608b6832/attachment.htm 


More information about the Amarok-devel mailing list